Why change semantics at this point. Wrapping to 0 itself would be a b/w incompatible change, no? For folks already handling this issue.
Isn't checking for negative just as easy (and unchanged from past history) as checking for 0? I believe 0 is the initial state, so negative is better from that perspective as well. Patrick On Sun, Apr 26, 2015 at 7:08 AM, Flavio Junqueira <[email protected]> wrote: > 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]> 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]> >> 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]> 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]> >> > 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]> wrote: >> > > >> > > >> > > >> > > I guess this is the mailing list discussion you are referring to: >> > > 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 >> > > >> > > >> > > >> > > On Wed, Apr 15, 2015 at 1:51 PM, Flavio Junqueira <[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 >> > > >> > > >> > > >> > > >> > >> > >> > >> > >> > >> > >> >> >> >> >> > > >
