zhaohaidao commented on code in PR #2064:
URL: https://github.com/apache/zookeeper/pull/2064#discussion_r1330333678
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/PrepRequestProcessor.java:
##########
@@ -735,10 +735,18 @@ private String getParentPathAndValidate(String path)
throws BadArgumentsExceptio
}
private static int checkAndIncVersion(int currentVersion, int
expectedVersion, String path) throws KeeperException.BadVersionException {
- if (expectedVersion != -1 && expectedVersion != currentVersion) {
- throw new KeeperException.BadVersionException(path);
+ if (expectedVersion != -1) {
+ if (expectedVersion != currentVersion) {
+ throw new KeeperException.BadVersionException(path);
+ }
+ if (expectedVersion == -2) {
+ return 0;
+ } else {
+ return currentVersion + 1;
+ }
+ } else {
+ return currentVersion + 1;
Review Comment:
Thanks for your detailed explanation. I almost understand the background.
> The next version should be 0 when currentVersion is -2 irrespective of
expectedVersion.
I'm not sure if my understanding of this sentence is correct, what we can
agree on is that the currentVersion needs to be changed from -2 to 0.
But if currentVersion=-2, the equality check is skipped, I am worried that
there may be safety risks?
For example, in the following scenario, two clients concurrently initiate
setData requests. Counted as C1 and C2 respectively, the version numbers
queried before C1 and C2 initiated the update were both -6, and then
simultaneously initiated requests to ZkServer. The request initiated by C2 was
successfully processed. C1 was delayed for several seconds due to network
problems. Here, During this period, C2 initiated three more requests, causing
the version number to become -2. Then the network between C1 and ZkServer
returned to normal. At this time, ZkServer did not perform an equality check
because currentVersion=-2, resulting in C1's request being successfully
processed.
Is this situation in line with expectations? My personal understanding is
that this is not in line with expectations for C2 and violates the semantics of
CAS, because C1 successfully updated with an older version.
--
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: notifications-unsubscr...@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
