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