I initiated this thread because I wanted to propose simply to wrap versions to 0 instead of getting negative numbers when it overflows. For versions at least, it doesn't seem to be problematic to wrap it to zero as long as no client holds a version number for too long. As part of that discussion, we started a parallel proposal to make some fields long to avoid overflows, which I believe would have the issue with BW that you've raised.
-Flavio > On 24 Apr 2015, at 06:44, Patrick Hunt <[email protected]> wrote: > > I was responding to this part of the thread (your/jordan's most recent > comments at the time) "What would be involved with changing these values to > longs so that the problem could be avoided now and into the future?" > > I was trying to say that if you're proposing a non-backward compatible change > (changing method signatures for example, int to long) then it would likely > only be possible in a major release. > > I don't see how you would "fix" w/o breaking backward compatibility. > > Patrick > > On Thu, Apr 23, 2015 at 8:06 AM, Flavio Junqueira > <[email protected] <mailto:[email protected]>> wrote: > Are you suggesting that we should start working on the 4.0 branch instead of > fixing this? I'm not clear on whether you're proposing a plan or just > throwing an idea, Pat. I'd like know so that I decide whether to go ahead and > propose a fix or if I should be targeting 4.0.x. > It'd be fascinating to work on the 4.0 branch, but having a release of such a > branch doesn't seem very realistic in the near future, we are still spending > all our effort on the 3.4/3.5 branches. > -Flavio > > > > On Friday, April 17, 2015 8:22 AM, Patrick Hunt <[email protected] > <mailto:[email protected]>> wrote: > > > > The main issue is ensuring backward compat, we'd need a major version in > order to address. We discussed in the past that when we move to 4.x we > would address breaking changes like moving from integers to longs for > things like version. > > That said we have also talked about having a new api using the longs, and a > "legacy" api maintaining the integers. We could for example implement the > "old" api in terms of the new api -- old code could use the legacy/integers > while new code could use the apis with longs. > > We'd also have to change the on disk format, so while I'm sure we could > provide the ability to upgrade to the new version, rolling back would not > be possible. > > Patrick > > On Thu, Apr 16, 2015 at 9:40 AM, Flavio Junqueira < > [email protected]> wrote: > > > Yeah, I thought about it, but the API change (at least for version) made > > me put it aside. Internally, it is a change to the jute config plus some > > changes to signatures and some code. > > -Flavio > > > > > > On Thursday, April 16, 2015 5:34 PM, Jordan Zimmerman < > > [email protected] <mailto:[email protected]>> wrote: > > > > > > > > What would be involved with changing these values to longs so that the > > problem could be avoided now and into the future? > > > > -Jordan > > > > > > > > On April 16, 2015 at 10:29:09 AM, Flavio Junqueira > > ([email protected]) wrote: > > > > In the case of cversion, I'd say that we need to error out the request > > because wrapping around might break recipes that use the minimum. For > > version, we could wrap it to zero, but we could also error out just to keep > > it consistent with the behavior of the other fields (e.g., cversion). > > -Flavio > > > > > > On Thursday, April 16, 2015 10:14 AM, Michi Mutsuzaki <[email protected] > > <mailto:[email protected]>> > > wrote: > > > > > > > > How should we deal with cversions? we probably shouldn't wrap to zero > > since cversions are used for sequential nodes, right? > > > > On Thu, Apr 16, 2015 at 1:55 AM, Flavio Junqueira > > <[email protected]> wrote: > > > It is indeed related, Michi, thanks for the pointers. I was thinking > > that at least in the case of versions, we could just wrap it to zero rather > > than let it become negative. Since we only check for equality, wrapping it > > to zero when it hits Integer.MAX_VALUE should be sufficient. > > > I'll generate a patch for this. > > > -Flavio > > > > > > > > > On Thursday, April 16, 2015 12:59 AM, Michi Mutsuzaki < > > [email protected] <mailto:[email protected]>> wrote: > > > > > > > > > > > > I guess this is the mailing list discussion you are referring to: > > > http://s.apache.org/qMw <http://s.apache.org/qMw> , I couldn't find an > > > open JIRA for this. I > > > filed a similar issue for client xid: > > > https://issues.apache.org/jira/browse/ZOOKEEPER-1485 > > > <https://issues.apache.org/jira/browse/ZOOKEEPER-1485> > > > > > > > > > > > > On Wed, Apr 15, 2015 at 1:51 PM, Flavio Junqueira <[email protected] > > > <mailto:[email protected]>> > > wrote: > > >> I was checking checkAndIncVersion in PrepRequestProcessor and we > > currently > > >> don't do anything special for wrapping around the version counter of a > > znode > > >> in the case it reaches the max int value. The problem is that > > incrementing > > >> the max value will give us negative values, and in principle versions > > are > > >> non-negative values. > > >> > > >> It is unlikely that most applications will hit it ever, but I was > > wondering > > >> if this has ever been a problem to anyone, and if there is any jira > > created > > >> about it. I searched and couldn't find anything, but I do remember a > > >> discussion about counters overflowing some time back. > > >> > > >> I'd appreciate feedback here. > > >> > > >> Thanks, > > >> -Flavio > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
