[GitHub] [zookeeper] anmolnar commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data

2022-12-05 Thread GitBox
anmolnar commented on code in PR #1943: URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1039441532 ## zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/IPAuthenticationProvider.java: ## @@ -128,4 +131,18 @@ public boolean isValid(String id) {

[GitHub] [zookeeper] anmolnar commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data

2022-12-05 Thread GitBox
anmolnar commented on code in PR #1943: URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1039443157 ## zookeeper-server/src/test/java/org/apache/zookeeper/server/util/RateLimiterTest.java: ## @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation

[GitHub] [zookeeper] MikeEdgar commented on pull request #1917: ZOOKEEPER-4293: Lock Contention in ClientCnxnSocketNetty

2022-12-05 Thread GitBox
MikeEdgar commented on PR #1917: URL: https://github.com/apache/zookeeper/pull/1917#issuecomment-1338209065 @eolivelli , I've made a change that should address the NPE in the new test. Question.. I see errors locally around `org.apache.zookeeper.server.FollowerRequestProcessorTest`

[GitHub] [zookeeper] li4wang commented on pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data

2022-12-05 Thread GitBox
li4wang commented on PR #1943: URL: https://github.com/apache/zookeeper/pull/1943#issuecomment-1338868682 > One more thing: the doc mentions that this option is required for the Admin's snapshot command to work properly. If that's the case the Admin command should check whether this option

[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data

2022-12-05 Thread GitBox
li4wang commented on code in PR #1943: URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1040568525 ## zookeeper-server/src/test/java/org/apache/zookeeper/server/util/RateLimiterTest.java: ## @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation (ASF)

[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data

2022-12-05 Thread GitBox
li4wang commented on code in PR #1943: URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1040607436 ## zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md: ## @@ -1208,7 +1208,32 @@ property, when available, is noted below. The default value is