kezhuw commented on code in PR #478:
URL: https://github.com/apache/curator/pull/478#discussion_r1311342375
##########
curator-recipes/src/main/java/org/apache/curator/framework/recipes/shared/SharedValue.java:
##########
@@ -196,7 +196,7 @@ public boolean trySetValue(VersionedValue<byte[]> previous,
byte[] newValue) thr
private void updateValue(int version, byte[] bytes) {
while (true) {
VersionedValue<byte[]> current = currentValue.get();
- if (current.getVersion() >= version) {
+ if (current.getVersion() >= version && version !=
Integer.MIN_VALUE) {
Review Comment:
Seems that
[`VersionedValue.version`](https://github.com/apache/curator/blob/72beebc504d90338ded9a68ae0f6b79f5c8986fd/curator-recipes/src/main/java/org/apache/curator/framework/recipes/shared/VersionedValue.java#L27)
are `Stat.version` [from
ZooKeeper](https://github.com/apache/curator/blob/72beebc504d90338ded9a68ae0f6b79f5c8986fd/curator-recipes/src/main/java/org/apache/curator/framework/recipes/shared/SharedValue.java#L251).
Then it is "known" to overflow in finite time in possible large `setData`
frequency. Given `default` visibility of `VersionedValue`'s visibility, I think
we probably can enhance this a bit with extra `zxid`. But the crucial problem
comes from ZooKeeper, it only do atomic check against `Stat.version` which is a
32-bit integer and overflow finally for long running frequently modified data.
And we will finally run into corner case
[`checkAndIncVersion`](https://github.com/apache/zookeeper/blob/03a36d08e257c43e8377e5549d5524805fc6b8bb/zookeeper-server/src/main/j
ava/org/apache/zookeeper/server/PrepRequestProcessor.java#L737) exposed, `-1`
is not an atomic condition which means we could write wrong node data in case
of wraparound and contention.
I really hope a `check_zxid` style `setData`. I am always fearing of
`setData` with `version` from old incarnation.
There is a similar case in
https://lists.apache.org/thread/4o3rl49rdj5y0134df922zgc8clyt86s, which
overflow `Stat.cversion`. I believed ZooKeeper was not designed and suitable
for certain usages, performance degradation is desireable in wrong usages. But
these overflow more sounds like bugs.
--
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]
