kezhuw commented on code in PR #478:
URL: https://github.com/apache/curator/pull/478#discussion_r1327946265
##########
curator-recipes/src/main/java/org/apache/curator/framework/recipes/shared/SharedValue.java:
##########
@@ -196,8 +196,12 @@ 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) {
- // A newer version was concurrently set.
Review Comment:
I think we should refactor `updateValue` a bit to compare ordering using
`Stat.mzxid`. This way we are not fearing this overflow issue. I am stupid in
reviwing without a deep thought, sorry for that. So my finally points are:
* Deprecate `VersionedValue#getVersion` to warn clients about "ordering
assumptions" and "overflow behavior".
* Refactor `updateValue` to order using `Stat.mzxid`.
* Throw exception in case of `-1` `Stat.version` in `trySetValue`. I am
positive to
[ZOOKEEPER-4743](https://issues.apache.org/jira/browse/ZOOKEEPER-4743).
* Document somehow about "overflow" and exception case in `trySetValue`.
Besides above, should we expose a `VersionedValue#getZxid` for client usage ?
Any thoughts @tisonkun @eolivelli @Hexiaoqiao ?
--
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]
