[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data
li4wang commented on code in PR #1943:
URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1040657179
##
zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/IPAuthenticationProvider.java:
##
@@ -128,4 +131,18 @@ public boolean isValid(String id) {
return true;
}
+/**
+ * Returns the HTTP(s) client IP address
+ * @param request HttpServletRequest
+ * @return IP address
+ */
+public static String getClientIPAddress(final HttpServletRequest request) {
+// to handle the case that a HTTP(s) client connects via a proxy or
load balancer
+final String xForwardedForHeader =
request.getHeader(X_FORWARDED_FOR_HEADER_NAME);
+if (xForwardedForHeader == null) {
+return request.getRemoteAddr();
+}
+// the format of the field is: X-Forwarded-For: client, proxy1, proxy2
...
+return new StringTokenizer(xForwardedForHeader,
",").nextToken().trim();
+}
Review Comment:
Yep
--
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
[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data
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 false. - +* *serializeLastProcessedZxid.enabled* + (Jave system property: **zookeeper.serializeLastProcessedZxid.enabled**) Review Comment: Yes, you are right. Added the check and also the unit test. -- 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
[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data
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) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.server.util;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+import org.junit.jupiter.api.Test;
+
+public class RateLimiterTest {
+
+@Test
+public void testAllow_withinInterval() {
+final int rate = 2;
+final RateLimiter rateLimiter = new RateLimiter(rate, 5,
TimeUnit.SECONDS);
+for (int i = 0; i < rate; i++) {
+assertTrue(rateLimiter.allow());
+}
+assertFalse(rateLimiter.allow());
+}
+
+@Test
+public void testAllow_withinInterval_multiThreaded() {
+final int rate = 10;
+
+final RateLimiter rateLimiter = new RateLimiter(rate, 5,
TimeUnit.SECONDS);
+final ThreadPoolExecutor executor = (ThreadPoolExecutor)
Executors.newFixedThreadPool(rate + 1);
+for (int i = 0; i < rate; i++) {
+executor.execute(() -> assertTrue(rateLimiter.allow()));
+}
+executor.execute(() -> assertFalse(rateLimiter.allow()));
+}
+
+@Test
+public void testAllow_exceedInterval() throws Exception {
+final int interval = 1;
+
+final RateLimiter rateLimiter = new RateLimiter(1, interval,
TimeUnit.SECONDS);
+assertTrue(rateLimiter.allow());
+assertFalse(rateLimiter.allow());
+Thread.sleep(TimeUnit.SECONDS.toMillis(interval) + 3);
Review Comment:
Good catch! I didn't realize the original issue was 1 ms more instead of 1
second more. Fixed.
--
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
[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data
li4wang commented on code in PR #1943: URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1038390377 ## zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md: ## @@ -1208,7 +1208,32 @@ property, when available, is noted below. The default value is false. - +* *serializeLastProcessedZxid.enabled* + (Jave system property: **zookeeper.serializeLastProcessedZxid.enabled**) Review Comment: No, I don't see anyone wants to turn it off unless we run into unexpected issue, and I think it doesn't hurt to have the option available for now. -- 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
[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data
li4wang commented on code in PR #1943: URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1038390377 ## zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md: ## @@ -1208,7 +1208,32 @@ property, when available, is noted below. The default value is false. - +* *serializeLastProcessedZxid.enabled* + (Jave system property: **zookeeper.serializeLastProcessedZxid.enabled**) Review Comment: No, I don't see anyone wants to turn it off unless we run into unexpected issue, but I feel it doesn't hurt to have the option available for now. -- 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
[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data
li4wang commented on code in PR #1943: URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1038390377 ## zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md: ## @@ -1208,7 +1208,32 @@ property, when available, is noted below. The default value is false. - +* *serializeLastProcessedZxid.enabled* + (Jave system property: **zookeeper.serializeLastProcessedZxid.enabled**) Review Comment: No, I don't see anyone wants to turn it off unless we run into unexpected issue, but it doesn't hurt to have optional available for now. -- 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
[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data
li4wang commented on code in PR #1943:
URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1038469137
##
zookeeper-server/src/test/java/org/apache/zookeeper/server/util/RateLimiterTest.java:
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.server.util;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.util.concurrent.TimeUnit;
+import org.junit.jupiter.api.Test;
+
+public class RateLimiterTest {
+
+@Test
+public void testAllow_withinInterval() {
+final int rate = 2;
+final RateLimiter rateLimiter = new RateLimiter(rate, 5,
TimeUnit.SECONDS);
+for (int i = 0; i < rate; i++) {
+assertTrue(rateLimiter.allow());
+}
+assertFalse(rateLimiter.allow());
+}
+
+@Test
+public void testAllow_exceedInterval() throws Exception {
+final int interval = 1;
+
+final RateLimiter rateLimiter = new RateLimiter(1, interval,
TimeUnit.SECONDS);
+assertTrue(rateLimiter.allow());
+assertFalse(rateLimiter.allow());
+Thread.sleep(TimeUnit.SECONDS.toMillis(interval) + 1);
Review Comment:
> Please wait 2 seconds at least. Thread.sleep() in tests could easly lead
to flaky test.
changed to wait for 3 seconds
> Additonally I think it would be nice to have a multithreaded test as well:
start 10 worker in an executor to grab the numbers and verify that the 11th is
disallowed within the time interval.
added multithreaded test
--
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
[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data
li4wang commented on code in PR #1943:
URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1038424755
##
zookeeper-server/src/main/java/org/apache/zookeeper/server/util/RateLimiter.java:
##
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.server.util;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+/**
+ * A class that provides simple interval-based rate limiting implementation.
+ */
+public class RateLimiter {
+private final int rate;
+private final long intervalInMs;
+private long lastTimeReset;
+private final AtomicInteger remained;
+
+public RateLimiter(final int rate, final long interval, final TimeUnit
unit) {
+this.rate = rate;
+this.intervalInMs = unit.toMillis(interval);
+this.lastTimeReset = System.currentTimeMillis();
Review Comment:
Good idea. Changed.
--
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
[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data
li4wang commented on code in PR #1943:
URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1038419979
##
zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java:
##
@@ -496,6 +506,87 @@ public CommandResponse run(ZooKeeperServer zkServer,
Map kwargs)
}
+/**
+ * Take a snapshot of current server and stream out the data.
+ *
+ * Argument:
+ * - "streaming": optional String to indicate whether streaming out data
+ *
+ * Returned snapshot as stream if streaming is true and metadata of the
snapshot
+ * - "last_zxid": String
+ * - "snapshot_size": String
+ */
+public static class SnapshotCommand extends CommandBase {
+static final String REQUEST_QUERY_PARAM_STREAMING = "streaming";
+
+static final String RESPONSE_HEADER_LAST_ZXID = "last_zxid";
+static final String RESPONSE_HEADER_SNAPSHOT_SIZE = "snapshot_size";
+
+static final String ADMIN_SNAPSHOT_ENABLED =
"zookeeper.admin.snapshot.enabled";
+static final String ADMIN_SNAPSHOT_INTERVAL =
"zookeeper.admin.snapshot.intervalInMS";
+
+private static final long snapshotInterval =
Integer.parseInt(System.getProperty(ADMIN_SNAPSHOT_INTERVAL, "30"));
+
+private final RateLimiter rateLimiter;
+
+public SnapshotCommand() {
+super(Arrays.asList("snapshot", "snap"));
+rateLimiter = new RateLimiter(1, snapshotInterval,
TimeUnit.MICROSECONDS);
+}
+
+@SuppressFBWarnings(value = "OBL_UNSATISFIED_OBLIGATION",
+justification = "FileInputStream is passed to
CommandResponse and closed in StreamOutputter")
+@Override
+public CommandResponse run(final ZooKeeperServer zkServer, final
Map kwargs) {
+final CommandResponse response = initializeResponse();
+
+// check feature flag
+final boolean snapshotEnabled =
Boolean.parseBoolean(System.getProperty(ADMIN_SNAPSHOT_ENABLED, "false"));
+if (!snapshotEnabled) {
+
response.setStatusCode(HttpServletResponse.SC_SERVICE_UNAVAILABLE);
+LOG.warn("Snapshot command is disabled");
+return response;
+}
+
+// check rate limiting
+if (!rateLimiter.allow()) {
+response.setStatusCode(429);
Review Comment:
Yes, changed.
--
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
[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data
li4wang commented on code in PR #1943:
URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1038417361
##
zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/StreamOutputter.java:
##
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.server.admin;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.zookeeper.common.IOUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A class for streaming data out.
+ */
+public class StreamOutputter implements CommandOutputter{
+private static final Logger LOG =
LoggerFactory.getLogger(StreamOutputter.class);
+private final String clientIP;
+
+public StreamOutputter(final String clientIP) {
+this.clientIP = clientIP;
+}
+
+@Override
+public String getContentType() {
+return "application/octet-stream";
+}
+
+@Override
+public void output(final CommandResponse response, final OutputStream os) {
+try (final InputStream is = response.getInputStream()){
+IOUtils.copyBytes(is, os, 1024, true);
+} catch (final IOException e) {
+LOG.error("Exception occurred when streaming out data to {}",
clientIP, e);
Review Comment:
Yeah, I think it's useful to add it to `JsonOutputter` too. Added.
--
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
[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data
li4wang commented on code in PR #1943:
URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1038401440
##
zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/IPAuthenticationProvider.java:
##
@@ -128,4 +131,18 @@ public boolean isValid(String id) {
return true;
}
+/**
+ * Returns the HTTP(s) client IP address
+ * @param request HttpServletRequest
+ * @return IP address
+ */
+public static String getClientIPAddress(final HttpServletRequest request) {
+// to handle the case that a HTTP(s) client connects via a proxy or
load balancer
+final String xForwardedForHeader =
request.getHeader(X_FORWARDED_FOR_HEADER_NAME);
+if (xForwardedForHeader == null) {
+return request.getRemoteAddr();
+}
+// the format of the field is: X-Forwarded-For: client, proxy1, proxy2
...
+return new StringTokenizer(xForwardedForHeader,
",").nextToken().trim();
+}
Review Comment:
This method is in `IPAuthenticationProvider `as it will be used by
`IPAuthenticationProvider` when providing IP auth support for admin server
APIs.
`IPAuthenticationProvider` will be enhanced for authenticating both
Zookeeper client and Admin Server APIs by the client IP address.
--
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
[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data
li4wang commented on code in PR #1943: URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1038390377 ## zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md: ## @@ -1208,7 +1208,32 @@ property, when available, is noted below. The default value is false. - +* *serializeLastProcessedZxid.enabled* + (Jave system property: **zookeeper.serializeLastProcessedZxid.enabled**) Review Comment: No, I don't see anyone wants to turn it off unless we run into unexpected issue. -- 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
[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data
li4wang commented on code in PR #1943:
URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1020630119
##
zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/SnapshotCommandTest.java:
##
@@ -0,0 +1,233 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.server.admin;
+
+import static
org.apache.zookeeper.server.admin.Commands.SnapshotCommand.ADMIN_SNAPSHOT_ENABLED;
+import static
org.apache.zookeeper.server.admin.Commands.SnapshotCommand.ADMIN_SNAPSHOT_INTERVAL;
+import static
org.apache.zookeeper.server.admin.Commands.SnapshotCommand.REQUEST_QUERY_PARAM_STREAMING;
+import static
org.apache.zookeeper.server.admin.JettyAdminServerTest.URL_FORMAT;
+import static
org.apache.zookeeper.server.admin.JettyAdminServerTest.jettyAdminPort;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.UnsupportedEncodingException;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.net.URLEncoder;
+import java.nio.file.Files;
+import java.util.HashMap;
+import java.util.Map;
+import javax.servlet.http.HttpServletResponse;
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.PortAssignment;
+import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.common.IOUtils;
+import org.apache.zookeeper.server.ServerCnxnFactory;
+import org.apache.zookeeper.server.ZooKeeperServer;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.io.TempDir;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+public class SnapshotCommandTest extends ZKTestCase {
+private static final Logger LOG =
LoggerFactory.getLogger(SnapshotCommandTest.class);
+
+private static final String PATH = "/snapshot_test";
+private static final int NODE_COUNT = 10;
+
+private final String hostPort = "127.0.0.1:" + PortAssignment.unique();
+private ServerCnxnFactory cnxnFactory;
+private JettyAdminServer adminServer;
+private ZooKeeperServer zks;
+private ZooKeeper zk;
+
+@TempDir
+static File dataDir;
+
+@TempDir
+static File logDir;
+
+@BeforeAll
+public void setup() throws Exception {
+// start ZookeeperServer
+System.setProperty("zookeeper.4lw.commands.whitelist", "*");
Review Comment:
Yes, all the system properties are cleared.
--
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
[GitHub] [zookeeper] li4wang commented on a diff in pull request #1943: ZOOKEEPER-4570: Admin server API for taking snapshot and stream out data
li4wang commented on code in PR #1943:
URL: https://github.com/apache/zookeeper/pull/1943#discussion_r1020629923
##
zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/StreamOutputter.java:
##
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.server.admin;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import org.apache.zookeeper.common.IOUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A class for streaming data out.
+ */
+public class StreamOutputter implements CommandOutputter{
+private static final Logger LOG =
LoggerFactory.getLogger(StreamOutputter.class);
+
+public StreamOutputter() {
+}
+
+@Override
+public String getContentType() {
+return "application/octet-stream";
+}
+
+@Override
+public void output(final CommandResponse response, final OutputStream os) {
+try (final InputStream is = response.getInputStream()){
+IOUtils.copyBytes(is, os, 1024, true);
+} catch (final IOException e) {
+LOG.error("Exception occurred when streaming out data", e);
Review Comment:
Good point. Enhanced.
--
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
