This is an automated email from the ASF dual-hosted git repository.
eolivelli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 5823a3f78 ZOOKEEPER-4743: Increase data version once more when going
back to -1 from Integer.MIN_VALUE (#2064)
5823a3f78 is described below
commit 5823a3f78b3ae032385a8b6f7b7594b44cf3a25a
Author: zhaohaidao <[email protected]>
AuthorDate: Sat Sep 30 18:22:21 2023 +0800
ZOOKEEPER-4743: Increase data version once more when going back to -1 from
Integer.MIN_VALUE (#2064)
Co-authored-by: zhaohaiyuan <[email protected]>
Co-authored-by: tison <[email protected]>
---
.../java/org/apache/zookeeper/server/DataTree.java | 14 +++
.../zookeeper/server/PrepRequestProcessor.java | 13 ++-
.../zookeeper/server/PrepRequestProcessorTest.java | 108 +++++++++++++++++++++
3 files changed, 134 insertions(+), 1 deletion(-)
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java
index b99bf632d..6dbe94332 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java
@@ -1962,4 +1962,18 @@ public class DataTree {
stat.setEphemeralOwner(ephemeralOwner);
return stat;
}
+
+ // for test only
+ static StatPersisted createStat(int version) {
+ StatPersisted stat = new StatPersisted();
+ stat.setCtime(0);
+ stat.setMtime(0);
+ stat.setCzxid(0);
+ stat.setMzxid(0);
+ stat.setPzxid(0);
+ stat.setVersion(version);
+ stat.setAversion(0);
+ stat.setEphemeralOwner(0);
+ return stat;
+ }
}
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/PrepRequestProcessor.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/PrepRequestProcessor.java
index 28d1cc97b..274862ee3 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/PrepRequestProcessor.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/PrepRequestProcessor.java
@@ -738,7 +738,18 @@ public class PrepRequestProcessor extends
ZooKeeperCriticalThread implements Req
if (expectedVersion != -1 && expectedVersion != currentVersion) {
throw new KeeperException.BadVersionException(path);
}
- return currentVersion + 1;
+ // Increase once more when going back to -1 from Integer.MIN_VALUE.
Otherwise, the client will
+ // receive a new data version -1. And Now, if the client wants to
check the data version, it can
+ // only pass -1 as the next expected version, but -1 as the expected
version means do not check
+ // the data version. So the client is unable to express the expected
manner.
+ //
+ // See also https://issues.apache.org/jira/browse/ZOOKEEPER-4743.
+ int nextVersion = currentVersion + 1;
+ if (nextVersion == -1) {
+ return 0;
+ } else {
+ return nextVersion;
+ }
}
/**
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/server/PrepRequestProcessorTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/PrepRequestProcessorTest.java
index b10258fb5..e24164158 100644
---
a/zookeeper-server/src/test/java/org/apache/zookeeper/server/PrepRequestProcessorTest.java
+++
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/PrepRequestProcessorTest.java
@@ -45,6 +45,7 @@ import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.ZooDefs.Ids;
import org.apache.zookeeper.ZooDefs.OpCode;
import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.data.Stat;
import org.apache.zookeeper.proto.CreateRequest;
import org.apache.zookeeper.proto.ReconfigRequest;
import org.apache.zookeeper.proto.RequestHeader;
@@ -59,6 +60,7 @@ import org.apache.zookeeper.server.quorum.QuorumPeerConfig;
import org.apache.zookeeper.server.quorum.flexible.QuorumVerifier;
import org.apache.zookeeper.test.ClientBase;
import org.apache.zookeeper.txn.ErrorTxn;
+import org.apache.zookeeper.txn.SetDataTxn;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -391,4 +393,110 @@ public class PrepRequestProcessorTest extends ClientBase {
return Collections.emptySet();
}
}
+
+ @Test
+ public void testCheckAndIncVersion() throws Exception {
+ zks.getZKDatabase().dataTree.createNode("/foo", new byte[0],
Ids.OPEN_ACL_UNSAFE, 0, 0, 0, 0);
+ pLatch = new CountDownLatch(1);
+ processor = new PrepRequestProcessor(zks, new MyRequestProcessor());
+
+ SetDataRequest record = new SetDataRequest("/foo", new byte[0], 0);
+ Request req = createRequest(record, OpCode.setData, false);
+ processor.pRequest(req);
+ pLatch.await();
+ assertEquals(OpCode.setData, outcome.getHdr().getType());
+ assertTrue(outcome.getTxn() instanceof SetDataTxn);
+ SetDataTxn setDataTxn = (SetDataTxn) outcome.getTxn();
+ assertEquals(1, setDataTxn.getVersion());
+ }
+
+ @Test
+ public void testCheckAndIncVersionOverflow() throws Exception {
+ Stat customStat = new Stat();
+ customStat.setVersion(Integer.MAX_VALUE);
+ zks.getZKDatabase().dataTree.createNode("/foo", new byte[0],
Ids.OPEN_ACL_UNSAFE, 0, 0, 0, 0);
+ DataNode node = zks.getZKDatabase().dataTree.getNode("/foo");
+ node.stat = DataTree.createStat(Integer.MAX_VALUE);
+
+ pLatch = new CountDownLatch(1);
+ processor = new PrepRequestProcessor(zks, new MyRequestProcessor());
+
+ SetDataRequest record = new SetDataRequest("/foo", new byte[0],
Integer.MAX_VALUE);
+ Request req = createRequest(record, OpCode.setData, false);
+ processor.pRequest(req);
+ pLatch.await();
+ assertEquals(OpCode.setData, outcome.getHdr().getType());
+ assertTrue(outcome.getTxn() instanceof SetDataTxn);
+ SetDataTxn setDataTxn = (SetDataTxn) outcome.getTxn();
+ assertEquals(Integer.MIN_VALUE, setDataTxn.getVersion());
+ }
+ @Test
+ public void testCheckAndIncVersionWithNegativeNumber() throws Exception {
+ zks.getZKDatabase().dataTree.createNode("/foo", new byte[0],
Ids.OPEN_ACL_UNSAFE, 0, 0, 0, 0);
+ DataNode node = zks.getZKDatabase().dataTree.getNode("/foo");
+ node.stat = DataTree.createStat(Integer.MIN_VALUE);
+
+ pLatch = new CountDownLatch(1);
+ processor = new PrepRequestProcessor(zks, new MyRequestProcessor());
+
+ SetDataRequest record = new SetDataRequest("/foo", new byte[0],
Integer.MIN_VALUE);
+ Request req = createRequest(record, OpCode.setData, false);
+ processor.pRequest(req);
+ pLatch.await();
+ assertEquals(OpCode.setData, outcome.getHdr().getType());
+ assertTrue(outcome.getTxn() instanceof SetDataTxn);
+ SetDataTxn setDataTxn = (SetDataTxn) outcome.getTxn();
+ assertEquals(Integer.MIN_VALUE + 1, setDataTxn.getVersion());
+ }
+
+ @Test
+ public void testCheckAndIncToZeroFromNegativeTwo() throws Exception {
+ zks.getZKDatabase().dataTree.createNode("/foo", new byte[0],
Ids.OPEN_ACL_UNSAFE, 0, 0, 0, 0);
+ DataNode node = zks.getZKDatabase().dataTree.getNode("/foo");
+ node.stat = DataTree.createStat(-2);
+
+ pLatch = new CountDownLatch(1);
+ processor = new PrepRequestProcessor(zks, new MyRequestProcessor());
+
+ SetDataRequest record = new SetDataRequest("/foo", new byte[0], -2);
+ Request req = createRequest(record, OpCode.setData, false);
+ processor.pRequest(req);
+ pLatch.await();
+ assertEquals(OpCode.setData, outcome.getHdr().getType());
+ assertTrue(outcome.getTxn() instanceof SetDataTxn);
+ SetDataTxn setDataTxn = (SetDataTxn) outcome.getTxn();
+ assertEquals(0, setDataTxn.getVersion());
+ }
+
+ @Test
+ public void testCheckAndIncSkipEqualityCheck() throws Exception {
+ zks.getZKDatabase().dataTree.createNode("/foo", new byte[0],
Ids.OPEN_ACL_UNSAFE, 0, 0, 0, 0);
+ DataNode node = zks.getZKDatabase().dataTree.getNode("/foo");
+
+ pLatch = new CountDownLatch(1);
+ processor = new PrepRequestProcessor(zks, new MyRequestProcessor());
+
+ SetDataRequest record = new SetDataRequest("/foo", new byte[0], -1);
+ Request req = createRequest(record, OpCode.setData, false);
+ processor.pRequest(req);
+ pLatch.await();
+ assertEquals(OpCode.setData, outcome.getHdr().getType());
+ assertTrue(outcome.getTxn() instanceof SetDataTxn);
+ SetDataTxn setDataTxn = (SetDataTxn) outcome.getTxn();
+ assertEquals(1, setDataTxn.getVersion());
+ }
+
+ @Test
+ public void testCheckAndIncWithBadVersion() throws Exception {
+ zks.getZKDatabase().dataTree.createNode("/foo", new byte[0],
Ids.OPEN_ACL_UNSAFE, 0, 0, 0, 0);
+ pLatch = new CountDownLatch(1);
+ processor = new PrepRequestProcessor(zks, new MyRequestProcessor());
+
+ SetDataRequest record = new SetDataRequest("/foo", new byte[0], 1);
+ Request req = createRequest(record, OpCode.setData, false);
+ processor.pRequest(req);
+ pLatch.await();
+ assertEquals(OpCode.error, outcome.getHdr().getType());
+ assertEquals(KeeperException.Code.BADVERSION,
outcome.getException().code());
+ }
}
