[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871889#comment-15871889 ] ASF GitHub Bot commented on ZOOKEEPER-2700: --- GitHub user flier opened a pull request: https://github.com/apache/zookeeper/pull/180 ZOOKEEPER-2700 add command to take snapshot When cold backup or remote offline sync Zookeeper instances, we need the latest snapshot. Add a four letter `snap` command to force Zookeeper to generate snapshot. You can merge this pull request into a Git repository by running: $ git pull https://github.com/flier/zookeeper ZOOKEEPER-2700 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/180.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #180 commit e1dc4c94ea41a3fc0e5511fe1d69e02f27f4f0b8 Author: Flier Lu Date: 2017-02-17T13:48:19Z add command to take snapshot commit 77402134bf172566b69b0ffe72f19f18b93ec989 Author: Flier Lu Date: 2017-02-17T13:57:51Z add document > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871903#comment-15871903 ] Hadoop QA commented on ZOOKEEPER-2700: -- -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +0 tests included. The patch appears to be a documentation patch that doesn't require tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. -1 release audit. The applied patch generated 1 release audit warnings (more than the trunk's current 0 warnings). -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/349//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/349//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/349//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/349//console This message is automatically generated. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871902#comment-15871902 ] Hadoop QA commented on ZOOKEEPER-2700: -- -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +0 tests included. The patch appears to be a documentation patch that doesn't require tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. -1 release audit. The applied patch generated 1 release audit warnings (more than the trunk's current 0 warnings). -1 core tests. The patch failed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/348//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/348//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/348//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/348//console This message is automatically generated. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871927#comment-15871927 ] Hadoop QA commented on ZOOKEEPER-2700: -- -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +0 tests included. The patch appears to be a documentation patch that doesn't require tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/350//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/350//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/350//console This message is automatically generated. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871945#comment-15871945 ] Edward Ribeiro commented on ZOOKEEPER-2700: --- Hi [~flier], Some observations: 1) this issue is a duplicate of ZOOKEEPER-1729 so I am marking it as such. 2) based on discussions on ZOOKEEPER-1729 (please, read it) and the security problems related on ZOOKEEPER-2693, implementing this command as a 4lw is a big *NO*. I think it could be be implemented as a JMX command and/or a ServerAdmin command tough. Additional comments I will post on the GH review comment. Cheers! > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871959#comment-15871959
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r101775786
--- Diff:
src/java/main/org/apache/zookeeper/server/command/SnapCommand.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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.command;
+
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+public class SnapCommand extends AbstractFourLetterCommand {
+private static final Logger LOG =
LoggerFactory.getLogger(SnapCommand.class);
+
+SnapCommand(PrintWriter pw, ServerCnxn serverCnxn) {
+super(pw,serverCnxn);
+}
+
+@Override
+public void commandRun() throws IOException {
+if (!isZKServerRunning()) {
+pw.println(ZK_NOT_SERVING);
+} else {
+Thread snapInProcess = new ZooKeeperThread("Snapshot Thread") {
--- End diff --
Wouldn't this also be a potential DoS attack? Especially with launching a
new thread in the background for the snapshot. Could we put in some sort of
throttling on taking the snapshot, so that we don't do it too frequently?
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871986#comment-15871986
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r101779855
--- Diff:
src/java/main/org/apache/zookeeper/server/command/SnapCommand.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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.command;
+
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+public class SnapCommand extends AbstractFourLetterCommand {
+private static final Logger LOG =
LoggerFactory.getLogger(SnapCommand.class);
+
+SnapCommand(PrintWriter pw, ServerCnxn serverCnxn) {
+super(pw,serverCnxn);
+}
+
+@Override
+public void commandRun() throws IOException {
+if (!isZKServerRunning()) {
+pw.println(ZK_NOT_SERVING);
+} else {
+Thread snapInProcess = new ZooKeeperThread("Snapshot Thread") {
--- End diff --
I think so too. Commented on the JIRA about that.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872002#comment-15872002
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r101782717
--- Diff:
src/java/main/org/apache/zookeeper/server/command/SnapCommand.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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.command;
+
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+public class SnapCommand extends AbstractFourLetterCommand {
+private static final Logger LOG =
LoggerFactory.getLogger(SnapCommand.class);
+
+SnapCommand(PrintWriter pw, ServerCnxn serverCnxn) {
+super(pw,serverCnxn);
+}
+
+@Override
+public void commandRun() throws IOException {
+if (!isZKServerRunning()) {
+pw.println(ZK_NOT_SERVING);
+} else {
+Thread snapInProcess = new ZooKeeperThread("Snapshot Thread") {
+public void run() {
+try {
+zkServer.takeSnapshot();
--- End diff --
Couldn't this call _potentially_ enter on a race condition with this
snippet below?
https://github.com/apache/zookeeper/blob/ec20c5434cc8a334b3fd25e27d26dccf4793c8f3/src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java#L132-L141
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872066#comment-15872066
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r101793125
--- Diff:
src/java/main/org/apache/zookeeper/server/command/SnapCommand.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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.command;
+
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+public class SnapCommand extends AbstractFourLetterCommand {
+private static final Logger LOG =
LoggerFactory.getLogger(SnapCommand.class);
+
+SnapCommand(PrintWriter pw, ServerCnxn serverCnxn) {
+super(pw,serverCnxn);
+}
+
+@Override
+public void commandRun() throws IOException {
+if (!isZKServerRunning()) {
+pw.println(ZK_NOT_SERVING);
+} else {
+Thread snapInProcess = new ZooKeeperThread("Snapshot Thread") {
--- End diff --
Since this PR is targeting master I suggest considering the option of
adding a snap API to ZooKeeperAdmin, which is recently introduced to harden
security around dynamic reconfiguration. ZooKeeperAdmin supports all sorts of
authentications built in ZK and we can extend it such that only admin (or any
users that explicitly being granted admin access to cluster) can issue snap
command.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872068#comment-15872068
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r101793204
--- Diff:
src/java/main/org/apache/zookeeper/server/command/SnapCommand.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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.command;
+
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+public class SnapCommand extends AbstractFourLetterCommand {
+private static final Logger LOG =
LoggerFactory.getLogger(SnapCommand.class);
+
+SnapCommand(PrintWriter pw, ServerCnxn serverCnxn) {
+super(pw,serverCnxn);
+}
+
+@Override
+public void commandRun() throws IOException {
+if (!isZKServerRunning()) {
+pw.println(ZK_NOT_SERVING);
+} else {
+Thread snapInProcess = new ZooKeeperThread("Snapshot Thread") {
--- End diff --
Also general comments about adding features to ZK: when you add a new
feature, please also add tests :)
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872363#comment-15872363
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r101829550
--- Diff:
src/java/main/org/apache/zookeeper/server/command/SnapCommand.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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.command;
+
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+public class SnapCommand extends AbstractFourLetterCommand {
+private static final Logger LOG =
LoggerFactory.getLogger(SnapCommand.class);
+
+SnapCommand(PrintWriter pw, ServerCnxn serverCnxn) {
+super(pw,serverCnxn);
+}
+
+@Override
+public void commandRun() throws IOException {
+if (!isZKServerRunning()) {
+pw.println(ZK_NOT_SERVING);
+} else {
+Thread snapInProcess = new ZooKeeperThread("Snapshot Thread") {
--- End diff --
@hanm @revans2 cool we are on the same page:
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?focusedCommentId=15871945&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15871945
:smile:
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872391#comment-15872391
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r101831774
--- Diff:
src/java/main/org/apache/zookeeper/server/command/SnapCommand.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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.command;
+
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+public class SnapCommand extends AbstractFourLetterCommand {
+private static final Logger LOG =
LoggerFactory.getLogger(SnapCommand.class);
+
+SnapCommand(PrintWriter pw, ServerCnxn serverCnxn) {
+super(pw,serverCnxn);
+}
+
+@Override
+public void commandRun() throws IOException {
+if (!isZKServerRunning()) {
+pw.println(ZK_NOT_SERVING);
+} else {
+Thread snapInProcess = new ZooKeeperThread("Snapshot Thread") {
--- End diff --
@hanm FYI, I suggested to add as a JMX option too.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872456#comment-15872456
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r101839349
--- Diff:
src/java/main/org/apache/zookeeper/server/command/SnapCommand.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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.command;
+
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+public class SnapCommand extends AbstractFourLetterCommand {
+private static final Logger LOG =
LoggerFactory.getLogger(SnapCommand.class);
+
+SnapCommand(PrintWriter pw, ServerCnxn serverCnxn) {
+super(pw,serverCnxn);
+}
+
+@Override
+public void commandRun() throws IOException {
+if (!isZKServerRunning()) {
+pw.println(ZK_NOT_SERVING);
+} else {
+Thread snapInProcess = new ZooKeeperThread("Snapshot Thread") {
--- End diff --
@eribeiro Yes that is one option - another would be Jetty AdminServer.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872478#comment-15872478
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user edwardoliveira commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r101841044
--- Diff:
src/java/main/org/apache/zookeeper/server/command/SnapCommand.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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.command;
+
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+public class SnapCommand extends AbstractFourLetterCommand {
+private static final Logger LOG =
LoggerFactory.getLogger(SnapCommand.class);
+
+SnapCommand(PrintWriter pw, ServerCnxn serverCnxn) {
+super(pw,serverCnxn);
+}
+
+@Override
+public void commandRun() throws IOException {
+if (!isZKServerRunning()) {
+pw.println(ZK_NOT_SERVING);
+} else {
+Thread snapInProcess = new ZooKeeperThread("Snapshot Thread") {
--- End diff --
Yup, suggested that one too. Either would be good, or both.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872671#comment-15872671
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user afine commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r101861654
--- Diff:
src/java/main/org/apache/zookeeper/server/command/SnapCommand.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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.command;
+
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+public class SnapCommand extends AbstractFourLetterCommand {
+private static final Logger LOG =
LoggerFactory.getLogger(SnapCommand.class);
+
+SnapCommand(PrintWriter pw, ServerCnxn serverCnxn) {
+super(pw,serverCnxn);
+}
+
+@Override
+public void commandRun() throws IOException {
+if (!isZKServerRunning()) {
+pw.println(ZK_NOT_SERVING);
+} else {
+Thread snapInProcess = new ZooKeeperThread("Snapshot Thread") {
+public void run() {
+try {
+zkServer.takeSnapshot();
--- End diff --
I agree. It may be nice to have some check that generally make sure
multiple snapshots cannot happen at the same time, as a sanity check.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872883#comment-15872883
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user flier commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r101880274
--- Diff:
src/java/main/org/apache/zookeeper/server/command/SnapCommand.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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.command;
+
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+public class SnapCommand extends AbstractFourLetterCommand {
+private static final Logger LOG =
LoggerFactory.getLogger(SnapCommand.class);
+
+SnapCommand(PrintWriter pw, ServerCnxn serverCnxn) {
+super(pw,serverCnxn);
+}
+
+@Override
+public void commandRun() throws IOException {
+if (!isZKServerRunning()) {
+pw.println(ZK_NOT_SERVING);
+} else {
+Thread snapInProcess = new ZooKeeperThread("Snapshot Thread") {
--- End diff --
@revans2 now, only generate snapshot when idle and last zxid changed.
@hanm I think `snap` should be a command to impact only one instance, no an
administration task to all instance in cluster.
@eribeiro added a `takeSnapshot` method to JMX
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872891#comment-15872891
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user flier commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r101880473
--- Diff:
src/java/main/org/apache/zookeeper/server/command/SnapCommand.java ---
@@ -0,0 +1,53 @@
+/**
+ * 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.command;
+
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+public class SnapCommand extends AbstractFourLetterCommand {
+private static final Logger LOG =
LoggerFactory.getLogger(SnapCommand.class);
+
+SnapCommand(PrintWriter pw, ServerCnxn serverCnxn) {
+super(pw,serverCnxn);
+}
+
+@Override
+public void commandRun() throws IOException {
+if (!isZKServerRunning()) {
+pw.println(ZK_NOT_SERVING);
+} else {
+Thread snapInProcess = new ZooKeeperThread("Snapshot Thread") {
+public void run() {
+try {
+zkServer.takeSnapshot();
--- End diff --
I have wrapped the manual requests from `snap` command or JMX with a
`tryTakeSnapshot` method, which will skip action when there are busy. If you
believe it is necessary, we could merge the check into `takeSnapshot`, which
may impact all the server workflow.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872899#comment-15872899 ] Hadoop QA commented on ZOOKEEPER-2700: -- +1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +0 tests included. The patch appears to be a documentation patch that doesn't require tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/352//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/352//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/352//console This message is automatically generated. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15874007#comment-15874007 ] Hadoop QA commented on ZOOKEEPER-2700: -- -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/355//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/355//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/355//console This message is automatically generated. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15874005#comment-15874005 ] ASF GitHub Bot commented on ZOOKEEPER-2700: --- Github user flier commented on the issue: https://github.com/apache/zookeeper/pull/180 According the review comments and [ZOOKEEPER-1729](https://issues.apache.org/jira/browse/ZOOKEEPER-1729), I have submited another patch to add JMX `takeSnapshot` method and Jetty Admin `snap` command to take snapshot, with test cases > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15874006#comment-15874006 ] Hadoop QA commented on ZOOKEEPER-2700: -- -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/354//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/354//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/354//console This message is automatically generated. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15874512#comment-15874512
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r101970085
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException,
InterruptedException {
public void takeSnapshot(){
try {
+lastSnapshotZxid = zkDb.getDataTreeLastProcessedZxid();
+isGeneratingSnapshot.incrementAndGet();
+
txnLogFactory.save(zkDb.getDataTree(),
zkDb.getSessionWithTimeOuts());
} catch (IOException e) {
LOG.error("Severe unrecoverable error, exiting", e);
// This is a severe error that we cannot recover from,
// so we need to exit
System.exit(10);
+} finally {
+isGeneratingSnapshot.decrementAndGet();
}
}
+public boolean tryTakeSnapshot() {
--- End diff --
In Java codebases we usually replace the "try" prefix by "maybe". ;)
So it becomes `maybeTakeSnapshot()`.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15874511#comment-15874511 ] ASF GitHub Bot commented on ZOOKEEPER-2700: --- Github user eribeiro commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/180#discussion_r101969629 --- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java --- @@ -126,6 +125,9 @@ private final ZooKeeperServerListener listener; private ZooKeeperServerShutdownHandler zkShutdownHandler; +private volatile long lastSnapshotZxid; +private AtomicInteger isGeneratingSnapshot = new AtomicInteger(0); --- End diff -- You could use an `AtomicBoolean` here, right? Make `isGeneratingSnapshot` **final**, please. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15874599#comment-15874599 ] ASF GitHub Bot commented on ZOOKEEPER-2700: --- Github user flier commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/180#discussion_r102028465 --- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java --- @@ -126,6 +125,9 @@ private final ZooKeeperServerListener listener; private ZooKeeperServerShutdownHandler zkShutdownHandler; +private volatile long lastSnapshotZxid; +private AtomicInteger isGeneratingSnapshot = new AtomicInteger(0); --- End diff -- In fact, I'm not sure whether Zookeeper backend threads will take snapshot in parallel. So, I choose to use a `AtomicInteger` to protect manual call `takeSnapshot`. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15874600#comment-15874600
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user flier commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r102028811
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException,
InterruptedException {
public void takeSnapshot(){
try {
+lastSnapshotZxid = zkDb.getDataTreeLastProcessedZxid();
+isGeneratingSnapshot.incrementAndGet();
+
txnLogFactory.save(zkDb.getDataTree(),
zkDb.getSessionWithTimeOuts());
} catch (IOException e) {
LOG.error("Severe unrecoverable error, exiting", e);
// This is a severe error that we cannot recover from,
// so we need to exit
System.exit(10);
+} finally {
+isGeneratingSnapshot.decrementAndGet();
}
}
+public boolean tryTakeSnapshot() {
--- End diff --
hmm...I haven't working on Java for years, forgive me :)
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15874621#comment-15874621 ] Hadoop QA commented on ZOOKEEPER-2700: -- -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/358//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/358//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/358//console This message is automatically generated. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15874623#comment-15874623 ] Hadoop QA commented on ZOOKEEPER-2700: -- -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The patch appears to cause tar ant target to fail. +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/359//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/359//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/359//console This message is automatically generated. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15874619#comment-15874619 ] Hadoop QA commented on ZOOKEEPER-2700: -- -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/356//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/356//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/356//console This message is automatically generated. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15874641#comment-15874641 ] Hadoop QA commented on ZOOKEEPER-2700: -- -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/360//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/360//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/360//console This message is automatically generated. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15874645#comment-15874645 ] Hadoop QA commented on ZOOKEEPER-2700: -- -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/361//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/361//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/361//console This message is automatically generated. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15874783#comment-15874783 ] Hadoop QA commented on ZOOKEEPER-2700: -- +1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/362//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/362//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/362//console This message is automatically generated. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876133#comment-15876133
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r102229805
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException,
InterruptedException {
public void takeSnapshot(){
--- End diff --
We now have a lot of potential paths to take a snapshot. JMX,
SyncRequestProcessor, Admin, and as part of the sync stage in ZABP. In the
past it was just the SyncRequestProcessor and ZABP that would trigger
snapshots. We didn't have much concern about collisions, because
SyncRequestProcessor would only run as edits were being sent, and the edits
would only be sent after the ZABP sync phase had completed. Now we have Admin
and JMX possibly doing a snapshot in the background.
Now if a snapshot request comes in during the ZABP sync phase after we have
clear out the in memory DB and not yet applied the new snapshot and then we
crash before we can write out the new snapshot we could end up with data
corruption. This should be super super rare so I don't really know if we care
all that much about it, but I think it is something that we can fix.
I am not an expert on the code, so I am not sure of the cleanest way to fix
this, but it feels like having maybeTakeSnapshot be a part of the
SyncRequestProcessor instead of ZooKeeperServer. I don't know how simple it is
to get to the SyncRequestProcessor from JMX/Admin but if you can then it means
that we can reset the edit count so we are not taking a lot of snapshots all at
once, one right after another. It also means that we can truly avoid having
more then one snapshot be taken at any time.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876134#comment-15876134
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r10749
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException,
InterruptedException {
public void takeSnapshot(){
try {
+lastSnapshotZxid = zkDb.getDataTreeLastProcessedZxid();
+isGeneratingSnapshot.incrementAndGet();
+
txnLogFactory.save(zkDb.getDataTree(),
zkDb.getSessionWithTimeOuts());
} catch (IOException e) {
LOG.error("Severe unrecoverable error, exiting", e);
// This is a severe error that we cannot recover from,
// so we need to exit
System.exit(10);
+} finally {
+isGeneratingSnapshot.decrementAndGet();
}
}
+public boolean maybeTakeSnapshot() {
--- End diff --
Could you add some javadocs here? It would be nice to explain the
difference between takeSnapshot and maybeTakeSnapshot.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876147#comment-15876147
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r102232941
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException,
InterruptedException {
public void takeSnapshot(){
--- End diff --
+1. This is the reason why I advocated the use of `AtomicBoolean` instead
of `AtomicInteger` to provide mutual exclusion on this code snippet.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876137#comment-15876137 ] ASF GitHub Bot commented on ZOOKEEPER-2700: --- Github user revans2 commented on the issue: https://github.com/apache/zookeeper/pull/180 Oh I also had the idea that it might be nice to provide feedback on the snap command for both JMX and the Admin command. You could provide more then just the last zxid, but you could also indicate if we are going to do the snapshot or not, and why. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876175#comment-15876175
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user flier commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r102238878
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException,
InterruptedException {
public void takeSnapshot(){
--- End diff --
If we are not expect to take snapshot on same time, the most easy way is to
use `AtomicBoolean ` protect the `takeSnapshot` from all the code path, which
may block the `SyncRequestProcessor` a while if a manual task is ongoing. My
current code is assuming the background `SyncRequestProcessor` have higher
priority.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876199#comment-15876199
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r102245094
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException,
InterruptedException {
public void takeSnapshot(){
--- End diff --
@eribeiro An AtomicBoolean is not enough as the SyncRequestProcessor
ignores it, only the JMX and admin commands use it.
@flier there are several things going on here and I don't know the
reasoning for all of them but I can guess.
1. Only take one snapshot at a time. I don't think this is super
critical, because it is not a correctness problem. Having multiple snapshots
happening at the same time should work correctly even in the face of crashes,
but it becomes a performance problem. There is a limited amount of CPU,
Memory, and most importantly disk bandwidth and IOps. The last successful
snapshot wins. So having multiple snapshots all going at the same time means
we are likely to be doing a lot of wasted work if everything does happen
successfully. If we can schedule the snapshots so there is only one going on
at a time then the full bandwidth and IOps can go to that snapshot. Even
better would be to space them out, so if we force a snapshot it makes
SyncRequestProcessor reset its counter so we don't take another one until X
more transactions have been processed.
2. Taking a snapshot at the wrong time and then crashing can corrupt the DB
on that node. This is potentially critical. The probability of this happening
is very very small, but if we can fix it so it can never happen, I would be
much happier.
I think we can fix both issues at the same time, by making JMX and Admin
use SyncRequestProcessor instead of going to the ZookeeperServer directly. If
no SyncRequestProcessor is ready then we can return such to the user so they
know the node is not ready to take a snapshot.
I also really would like to understand the use case for this command,
because I just don't see much value to a user to force another snapshot if one
is already in progress. I also would like to understand when you would want to
take a snapshot and if these changes would too severely limit that.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876305#comment-15876305
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user flier commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r102263924
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException,
InterruptedException {
public void takeSnapshot(){
--- End diff --
I have some scenes need to take snapshot, for example
First, our major Zookeeper cluster was deployed in an AWS zone, some
observers running at a dozen IDC. We use this topological structure because
Zookeeper cluster is not friendly to multi-IDC deployment. Besize, our
zookeeper snapshot and transaction logs are huge, because some wrong client
usage that hard to fix in short time :(
Sometimes, we plan to maintains the major cluster, we have to start another
mirror cluster in same DC, and switch from the major cluster to the mirror
cluster. If we do it faster enough, the observer and client will not concern
the changes. That's why we need take snapshot to speed up the migration. If
something got wrong, we could switch back to the old cluster, lost some
transaction better than the whole system down.
Second, our backup policy need a daily/hourly offline backup, to AWS S3 or
other DC. I would like to take and upload a latest and clean snapshot, instead
of tar an old snapshot with a number of transaction logs.
Third, sometimes we need to deploy a new observer or a testing cluster in
different DC, we have to copy the latest snapshot offline, because Zookeeper
observer sync progress may become very slow, the TCP window could drop to
10-20KB/s in the 40-60% packet loss rate.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876335#comment-15876335 ] ASF GitHub Bot commented on ZOOKEEPER-2700: --- Github user flier commented on the issue: https://github.com/apache/zookeeper/pull/180 @revans2 added a [generating](https://github.com/apache/zookeeper/pull/180/files#diff-8c1d5f61ac728fda8f4b706ae4277364R524) field to the response of `TakeSnapshotCommand ` command, and JMX `takeSnapshot` will return a boolean for same reason. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876385#comment-15876385
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user flier commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r102271596
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException,
InterruptedException {
public void takeSnapshot(){
--- End diff --
@revans2 It's not easy to find `SyncRequestProcessor `, but we may use
[reversed
workaroud](https://github.com/apache/zookeeper/pull/180/files#diff-a8615898f7be77e9cfdb686e21c97b1eL123),
let's `SyncRequestProcessor` use
`zks.getLastProcessedZxid()-zks.getLastSnapshotZxid()` to trace snapshot
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876386#comment-15876386 ] Hadoop QA commented on ZOOKEEPER-2700: -- -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/363//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/363//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/363//console This message is automatically generated. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876396#comment-15876396 ] Hadoop QA commented on ZOOKEEPER-2700: -- -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/364//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/364//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/364//console This message is automatically generated. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876403#comment-15876403 ] Hadoop QA commented on ZOOKEEPER-2700: -- -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/365//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/365//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/365//console This message is automatically generated. > Force ZooKeeper to generate snapshot > > > Key: ZOOKEEPER-2700 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700 > Project: ZooKeeper > Issue Type: Improvement >Reporter: Flier Lu >Priority: Minor > > When cold backup or remote offline sync Zookeeper instances, we need the > latest snapshot. > Add a four letter `snap` command to force Zookeeper to generate snapshot. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876447#comment-15876447
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r102282461
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException,
InterruptedException {
public void takeSnapshot(){
--- End diff --
Cross DC deployment is an interesting topic and ZooKeeper does not
intrinsically support it very well. Not saying that you don't need your snap
command (I understand it is a quick and dirty way to get things working for
your case), but here is some design that you might find useful for your
deployment:
https://www.usenix.org/system/files/conference/atc16/atc16_paper-lev-ari.pdf
The basic idea of this is to partition your data to have multiple ZK
ensembles (this loses global strong consistency) and then patch global
consistency at client side. The client library is open sourced somewhere.
https://issues.apache.org/jira/browse/ZOOKEEPER-892
This is an old issue that no one driving at moment but sounds a good fit
for your use case.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876461#comment-15876461
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r102283801
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException,
InterruptedException {
public void takeSnapshot(){
--- End diff --
@flier Having SyncRequestProcessor look at the number of snapshot files
feels like a bit of a hack, but I am not expert here. I would be fine with
having SyncRequestProcessor delegate all throttling of snapshots to
ZooKeeperServer, but then we need some sort of synchronization to prevent a
snapshot from being taken when it is not in a proper state (during the ZABP
sync phase).
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15877343#comment-15877343
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user flier commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r102372975
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException,
InterruptedException {
public void takeSnapshot(){
--- End diff --
@hanm Thanks for your advice :)
We are using a similar structure as `2.2 Alternative 2 – Learners` in the
paper, it is good enough for most of online scenes. I don't think it is worthy
to introduce another layer because we give up the write operation to all
observers, just use it as a read only view.
For the remote replication, I doubt it also blocked by packet loss rate
like Observer. On the other hand, we have an internal project named `zkpipe`,
it read Zookeeper snapshot/binlog and send it to a Kafka topic, our client
could choose to rebuild the transaction or subscribe the changes. I believe it
will better than hack Zookeeper itself. If you have interested, I could push it
to github later.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15900485#comment-15900485
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r104819835
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException,
InterruptedException {
public void takeSnapshot(){
--- End diff --
>> we have an internal project named zkpipe, it read Zookeeper
snapshot/binlog and send it to a Kafka topic, our client could choose to
rebuild the transaction or subscribe the changes. I believe it will better than
hack Zookeeper itself. If you have interested, I could push it to github later.
@flier This sounds interesting. I am sure there are users of ZooKeeper that
could benefit from this, because ZooKeeper does not work out of back for such
backup scenarios. If you are OK / allowed to open source this work I recommend
put it on github.
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2700) Force ZooKeeper to generate snapshot
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15900584#comment-15900584
]
ASF GitHub Bot commented on ZOOKEEPER-2700:
---
Github user flier commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/180#discussion_r104831719
--- Diff: src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java ---
@@ -303,15 +305,38 @@ public void loadData() throws IOException,
InterruptedException {
public void takeSnapshot(){
--- End diff --
@hanm sure, please check the [zkpipe](https://github.com/flier/zkpipe)
project, we are using it to tracing and audit zookeeper operations
> Force ZooKeeper to generate snapshot
>
>
> Key: ZOOKEEPER-2700
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2700
> Project: ZooKeeper
> Issue Type: Improvement
>Reporter: Flier Lu
>Priority: Minor
>
> When cold backup or remote offline sync Zookeeper instances, we need the
> latest snapshot.
> Add a four letter `snap` command to force Zookeeper to generate snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
