This is an automated email from the ASF dual-hosted git repository.
kezhuw 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 66f9cc394 ZOOKEEPER-2623: [ADDENDUM] Forbid OpCode.check outside
OpCode.multi
66f9cc394 is described below
commit 66f9cc3941f09ca00464760c78e30b97bd7c1faa
Author: Kezhu Wang <[email protected]>
AuthorDate: Mon Aug 5 21:49:33 2024 +0800
ZOOKEEPER-2623: [ADDENDUM] Forbid OpCode.check outside OpCode.multi
ZOOKEEPER-2623: [ADDENDUM] Forbid OpCode.check outside OpCode.multi
Individual `OpCode.check` will get `UnimplementedException`.
Reviewers: anmolnar, shoothzj, tisonkun
Author: kezhuw
Closes #2104 from
kezhuw/ZOOKEEPER-2623-addendum-forbid-opcode-check-outside-multi
---
.../zookeeper/server/FinalRequestProcessor.java | 20 ++------------------
.../zookeeper/server/PrepRequestProcessor.java | 4 ++++
.../java/org/apache/zookeeper/server/Request.java | 3 ++-
.../zookeeper/server/quorum/CommitProcessor.java | 1 +
.../server/quorum/FollowerRequestProcessor.java | 1 +
.../server/quorum/ObserverRequestProcessor.java | 1 +
.../server/quorum/ReadOnlyRequestProcessor.java | 1 +
.../server/util/RequestPathMetricsCollector.java | 1 +
.../java/org/apache/zookeeper/test/CheckTest.java | 13 +++----------
9 files changed, 16 insertions(+), 29 deletions(-)
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
index d3e20eca2..911583f9f 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
@@ -50,7 +50,6 @@ import org.apache.zookeeper.data.ACL;
import org.apache.zookeeper.data.Id;
import org.apache.zookeeper.data.Stat;
import org.apache.zookeeper.proto.AddWatchRequest;
-import org.apache.zookeeper.proto.CheckVersionRequest;
import org.apache.zookeeper.proto.CheckWatchesRequest;
import org.apache.zookeeper.proto.Create2Response;
import org.apache.zookeeper.proto.CreateResponse;
@@ -355,10 +354,8 @@ public class FinalRequestProcessor implements
RequestProcessor {
}
case OpCode.check: {
lastOp = "CHEC";
- CheckVersionRequest checkVersionRequest =
request.readRequestRecord(CheckVersionRequest::new);
- path = checkVersionRequest.getPath();
- handleCheckVersionRequest(checkVersionRequest, cnxn,
request.authInfo);
- requestPathMetricsCollector.registerRequest(request.type,
path);
+ rsp = new SetDataResponse(rc.stat);
+ err = Code.get(rc.err);
break;
}
case OpCode.exists: {
@@ -655,19 +652,6 @@ public class FinalRequestProcessor implements
RequestProcessor {
return new GetDataResponse(b, stat);
}
- private void handleCheckVersionRequest(CheckVersionRequest request,
ServerCnxn cnxn, List<Id> authInfo) throws KeeperException {
- String path = request.getPath();
- DataNode n = zks.getZKDatabase().getNode(path);
- if (n == null) {
- throw new KeeperException.NoNodeException();
- }
- zks.checkACL(cnxn, zks.getZKDatabase().aclForNode(n),
ZooDefs.Perms.READ, authInfo, path, null);
- int version = request.getVersion();
- if (version != -1 && version != n.stat.getVersion()) {
- throw new KeeperException.BadVersionException(path);
- }
- }
-
private boolean closeSession(ServerCnxnFactory serverCnxnFactory, long
sessionId) {
if (serverCnxnFactory == null) {
return false;
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 4666db53f..961948a9c 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
@@ -806,6 +806,10 @@ public class PrepRequestProcessor extends
ZooKeeperCriticalThread implements Req
SetACLRequest setAclRequest =
request.readRequestRecord(SetACLRequest::new);
pRequest2Txn(request.type, zks.getNextZxid(), request,
setAclRequest);
break;
+ case OpCode.check:
+ CheckVersionRequest checkRequest =
request.readRequestRecord(CheckVersionRequest::new);
+ pRequest2Txn(request.type, zks.getNextZxid(), request,
checkRequest);
+ break;
case OpCode.multi:
MultiOperationRecord multiRequest;
try {
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/Request.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/Request.java
index 27fa4e2df..c174fdd1e 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/Request.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/Request.java
@@ -295,8 +295,8 @@ public class Request {
// make sure this is always synchronized with Zoodefs!!
switch (type) {
case OpCode.notification:
- return false;
case OpCode.check:
+ return false;
case OpCode.closeSession:
case OpCode.create:
case OpCode.create2:
@@ -352,6 +352,7 @@ public class Request {
case OpCode.deleteContainer:
case OpCode.setACL:
case OpCode.setData:
+ case OpCode.check:
case OpCode.multi:
case OpCode.reconfig:
return true;
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java
index 0445bbdef..3be43c37e 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java
@@ -181,6 +181,7 @@ public class CommitProcessor extends
ZooKeeperCriticalThread implements RequestP
case OpCode.reconfig:
case OpCode.multi:
case OpCode.setACL:
+ case OpCode.check:
return true;
case OpCode.sync:
return matchSyncs;
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java
index 2798dd789..ef01ab322 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java
@@ -108,6 +108,7 @@ public class FollowerRequestProcessor extends
ZooKeeperCriticalThread implements
case OpCode.reconfig:
case OpCode.setACL:
case OpCode.multi:
+ case OpCode.check:
zks.getFollower().request(request);
break;
case OpCode.createSession:
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java
index 565f2778b..1d7e5c5c3 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java
@@ -109,6 +109,7 @@ public class ObserverRequestProcessor extends
ZooKeeperCriticalThread implements
case OpCode.reconfig:
case OpCode.setACL:
case OpCode.multi:
+ case OpCode.check:
zks.getObserver().request(request);
break;
case OpCode.createSession:
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java
index 7824af5cb..671d328ff 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java
@@ -85,6 +85,7 @@ public class ReadOnlyRequestProcessor extends
ZooKeeperCriticalThread implements
case OpCode.reconfig:
case OpCode.setACL:
case OpCode.multi:
+ case OpCode.check:
sendErrorResponse(request);
continue;
case OpCode.closeSession:
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/RequestPathMetricsCollector.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/RequestPathMetricsCollector.java
index e8f41e023..5332b1aae 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/RequestPathMetricsCollector.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/RequestPathMetricsCollector.java
@@ -147,6 +147,7 @@ public class RequestPathMetricsCollector {
case ZooDefs.OpCode.reconfig:
case ZooDefs.OpCode.setACL:
case ZooDefs.OpCode.multi:
+ case ZooDefs.OpCode.check:
return true;
}
return false;
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/test/CheckTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/CheckTest.java
index e81313273..0e2a1a4ec 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/CheckTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/CheckTest.java
@@ -77,14 +77,7 @@ public class CheckTest extends ClientBase {
private void testOperations(TestableZooKeeper zk) throws Exception {
Stat stat = new Stat();
zk.getData("/", false, stat);
- checkVersion(zk, "/", -1);
- checkVersion(zk, "/", stat.getVersion());
- assertThrows(KeeperException.BadVersionException.class, () -> {
- checkVersion(zk, "/", stat.getVersion() + 1);
- });
- assertThrows(KeeperException.NoNodeException.class, () -> {
- checkVersion(zk, "/no-node", Integer.MAX_VALUE);
- });
+ assertThrows(KeeperException.UnimplementedException.class, () ->
checkVersion(zk, "/", -1));
}
@Test
@@ -96,7 +89,7 @@ public class CheckTest extends ClientBase {
public void testStandaloneDatabaseReloadAfterCheck() throws Exception {
try {
testOperations(createClient());
- } catch (Exception ignored) {
+ } catch (Throwable ignored) {
// Ignore to test database reload after check
}
stopServer();
@@ -131,7 +124,7 @@ public class CheckTest extends ClientBase {
try {
testOperations(qb.createClient(new CountdownWatcher(),
QuorumPeer.ServerState.LEADING));
- } catch (Exception ignored) {
+ } catch (Throwable ignored) {
// Ignore to test database reload after check
}
qb.shutdown(leader);
