Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incr/decr parameter "by" should be a long, not an int #199

Closed
GoogleCodeExporter opened this issue Apr 7, 2016 · 9 comments
Closed

incr/decr parameter "by" should be a long, not an int #199

GoogleCodeExporter opened this issue Apr 7, 2016 · 9 comments

Comments

@GoogleCodeExporter
Copy link
Contributor

Is there a reason the method signature for inc and decr takes an int to 
represent the delta? 

The memcached protocol treats it as a "decimal representation of a 64-bit 
unsigned integer."

Limiting to a java int seems unnecessary.

Original issue reported on code.google.com by ori.schw...@gmail.com on 26 Aug 2011 at 4:28

@GoogleCodeExporter
Copy link
Contributor Author

There was a lot of discussion about this in the past.  We actually had a lot of 
discussion in the protocol definition about making it an *unsigned* 64-bit 
integer on the wire.  I still think that was a pretty awful mistake, but oh 
well.

In practice, I don't think people are likely to be bumping counters up by more 
than 2 billion at a go, but I won't argue that it's wrong.  It can't go in in a 
minor version, though, because it'll change the method signature.

Original comment by dsalli...@gmail.com on 26 Aug 2011 at 6:38

  • Changed state: Accepted

@GoogleCodeExporter
Copy link
Contributor Author

Thank you. I use one bit-shifted counter to store 3 different values so that's 
why I need to increment by a large values.

One option that might make it eligible for minor version release is if you 
added different methods for incr/decr by a long and the previous int version 
delegated to them.

Original comment by ori.schw...@gmail.com on 26 Aug 2011 at 3:41

@GoogleCodeExporter
Copy link
Contributor Author

Yeah, that I'd rather not do.  There are already way too many methods in there. 
 I've been hoping to get rid of the bulk of them for years.  :(

Your case makes sense, though.  I've not heard of anyone doing that before.

Original comment by dsalli...@gmail.com on 26 Aug 2011 at 4:50

@GoogleCodeExporter
Copy link
Contributor Author

Original comment by mikewie...@gmail.com on 29 Aug 2011 at 5:33

  • Added labels: Milestone-Release2.8

@GoogleCodeExporter
Copy link
Contributor Author

I have to disagree with dsallings on this.  Just changing it from int to long 
will suddenly break a lot of code.  I agree that there are too many methods in 
there, but shipping a library update that breaks existing binaries is just 
wrong, especially when it can be prevented.

As annoying as it might be, perhaps we need to update the interface, the class 
and include an abstract to implement this?

Sorry to be late to the discussion.

Original comment by ingen...@gmail.com on 7 Sep 2011 at 2:17

@GoogleCodeExporter
Copy link
Contributor Author

I was thinking about having it come in as a new version.  I don't want to just 
keep broadening the surface area.  The number of methods has already nearly 
tripled from when I tried to start bringing it down.  :/

If it is decided to do both, we should at least deprecate some of the stuff we 
don't want people using so we *can* get rid of it in the future.

Original comment by dsalli...@gmail.com on 7 Sep 2011 at 7:24

@GoogleCodeExporter
Copy link
Contributor Author

I agree that we should deprecate and remove in the future.  I just don't want 
people to be afraid to update to newer versions, which is what we may cause if 
we randomly take away functions.  People don't always get to rebuild from 
source.  Ops people sometimes need to update something to address a bug, 
without cracking open a project.

If this were rarely used functionality, I'd say we can break it and release 
note it, but incr is probably super common.

Original comment by ingen...@gmail.com on 7 Sep 2011 at 6:19

@GoogleCodeExporter
Copy link
Contributor Author

Original comment by mikewie...@gmail.com on 24 Oct 2011 at 10:13

  • Changed state: Fixed

@GoogleCodeExporter
Copy link
Contributor Author

Fixed in 2.8 developer preview 3

Original comment by mikewie...@gmail.com on 24 Oct 2011 at 10:14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant