fanyang89 commented on code in PR #1925:
URL: https://github.com/apache/zookeeper/pull/1925#discussion_r1152851048
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/SyncRequestProcessor.java:
##########
@@ -174,6 +224,21 @@ public void run() {
break;
}
+ if (si == turnForwardingDelayOn) {
+ nextProcessor.close();
Review Comment:
At the ctor of SyncRequestProcessor, nextProcessor may be null.
Can this be an NPE at ObserverZooKeeperServer(with
syncRequestProcessorEnabled=true)?
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ObserverZooKeeperServer.java:
##########
@@ -106,6 +106,8 @@ protected void setupRequestProcessors() {
if (syncRequestProcessorEnabled) {
syncProcessor = new SyncRequestProcessor(this, null);
syncProcessor.start();
+ } else {
+ syncProcessor = null;
Review Comment:
syncProcessor as an ObserverZooKeeperServer field should have a default
value of null.
Does setting null here makes a difference?
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/SyncRequestProcessor.java:
##########
@@ -174,6 +224,21 @@ public void run() {
break;
}
+ if (si == turnForwardingDelayOn) {
+ nextProcessor.close();
+ continue;
+ }
+ if (si == turnForwardingDelayOff) {
+ nextProcessor.open();
Review Comment:
This naming here is confusing.
The intention here is: to get the turning delaying off request, open the
gate, then flush all pending requests to the downstream processor.
nextProcessor.open() is to open the gate or turn the delay on?
--
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
