ctubbsii commented on code in PR #2737:
URL: https://github.com/apache/accumulo/pull/2737#discussion_r882717739
##########
server/base/src/main/java/org/apache/accumulo/server/conf/codec/VersionedProperties.java:
##########
@@ -106,10 +107,15 @@ public VersionedProperties(final int dataVersion, final
Instant timestamp,
* value should be used on data writes as the expected version. If the data
write fails do to an
* unexpected version, it signals that the node version has changed since
the instance was
* instantiated and encoded.
+ * <p>
+ * Implementation note: The data version is stored and returned is an
unsigned 32-bit integer
+ * value. Internally, ZooKeeper stores the value as a 32-bit signed value
that can roll-over and
+ * become negative. The can break applications that rely on the value to
always increase. This
+ * class ensures that the long returned will increase and will not be
negative.
Review Comment:
> Second part first - yes, it will potentially rollover back to zero.
In that case, the javadoc wording is suspect.
```suggestion
* become negative. The can break applications that rely on the value to
always increase. This
* class avoids rollover by doubling the positive values and ensures it
will never be negative.
```
> The update count in ZooBasedConfiguration expects that it will always
increase
That might be okay for now... but it would be nice to change that mechanism
so it can detect changes in the hierarchy without caring about the numeric
value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
