anmolnar commented on code in PR #1961:
URL: https://github.com/apache/zookeeper/pull/1961#discussion_r1064647933


##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java:
##########
@@ -1151,6 +1213,9 @@ protected void setLocalSessionFlag(Request si) {
     }
 
     public void submitRequest(Request si) {
+        if (state == State.MAINTENANCE) {
+            throw new IllegalStateException("Zookeeper server is in 
maintenance state");
+        }

Review Comment:
   If you throw an exception here, the request will fail on the client side. I 
haven't tried it myself, have you valdiated that the client is able to 
seemlessly retry the command without bothering the user?
   If not, I suggest another solution: create a semaphore which is closed while 
the maintenace is happening and block this thread until it's finished. The 
request will suffer some additionaly latency, but otherwise cannot be noticed. 
Thoughts?



##########
zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md:
##########
@@ -2126,6 +2126,17 @@ options are used to configure the 
[AdminServer](#sc_adminserver).
   The time interval for rate limiting snapshot command to protect the server.
   Defaults to 5 mins.
 
+* *admin.restore.enabled* :
+  (Java system property: **zookeeper.admin.restore.enabled**)
+  The flag for enabling the restore command. Defaults to false.
+  It will be enabled by default once the auth support for admin server commands
+  is available.
+
+* *admin.restore.intervalInMS* :
+  (Java system property: **zookeeper.admin.restore.intervalInMS**)
+  The time interval for rate limiting restore command to protect the server.
+  Defaults to 5 mins.

Review Comment:
   Do we need this setting separately for all admin commands?
   Shouldn't we just introduce a new general setting for the admin interface 
which would rate limit all admin requests coming in? 
   I don't think anybody would like to set this individually for commands.



-- 
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

Reply via email to