You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by sy...@apache.org on 2022/05/16 19:11:51 UTC
[zookeeper] branch branch-3.6 updated: ZOOKEEPER-3781: Create snapshots on followers when snapshot.trust.empty is true
This is an automated email from the ASF dual-hosted git repository.
symat pushed a commit to branch branch-3.6
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/branch-3.6 by this push:
new 20a52137e ZOOKEEPER-3781: Create snapshots on followers when snapshot.trust.empty is true
20a52137e is described below
commit 20a52137e06a937900f4a88b905ae7e37e5175ab
Author: Stig Rohde Døssing <st...@humio.com>
AuthorDate: Sat Mar 6 19:38:43 2021 +0000
ZOOKEEPER-3781: Create snapshots on followers when snapshot.trust.empty is true
snapshot.trust.empty is an escape hatch for users upgrading from 3.4.x to later Zookeeper
versions, allowing nodes to start with a non-empty transaction log but no snapshot.
The intent is for this setting to be enabled for a short while during the upgrade,
and then disabled again, as the check it disables is a safety feature.
Prior to this PR, a node would only write a snapshot locally if it became leader,
or if it had fallen so far behind the leader that the leader sent a SNAP message instead
of a DIFF. This made the upgrade process inconvenient, as not all nodes would create
a snapshot when snapshot.trust.empty was true, meaning that the safety check could
not be flipped back on.
This PR makes follower nodes write a local snapshot when they receive NEWLEADER,
if they have no local snapshot and snapshot.trust.empty is true.
Author: Stig Rohde Døssing <st...@humio.com>
Reviewers: Enrico Olivelli <eo...@apache.org>, Damien Diederen <dd...@apache.org>
Closes #1581 from srdo/zookeeper-3781
(cherry picked from commit 1214d3bf611d153ae8c3987523da01d3d6c82686)
Signed-off-by: Damien Diederen <dd...@apache.org>
(cherry picked from commit 9722488872802c546d1f8a3bb0ea349e5309f806)
---
.../apache/zookeeper/server/ZooKeeperServer.java | 4 ++
.../server/persistence/FileTxnSnapLog.java | 9 ++++
.../apache/zookeeper/server/quorum/Learner.java | 8 +++-
.../test/EmptiedSnapshotRecoveryTest.java | 50 ++++++++++++++++++++++
.../java/org/apache/zookeeper/test/QuorumUtil.java | 12 +++---
5 files changed, 76 insertions(+), 7 deletions(-)
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
index 6a58dc16f..16b62db22 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
@@ -530,6 +530,10 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider {
ServerMetrics.getMetrics().SNAPSHOT_TIME.add(elapsed);
}
+ public boolean shouldForceWriteInitialSnapshotAfterLeaderElection() {
+ return txnLogFactory.shouldForceWriteInitialSnapshotAfterLeaderElection();
+ }
+
@Override
public long getDataDirSize() {
if (zkDb == null) {
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
index eddeae8d9..c1d478270 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
@@ -229,6 +229,15 @@ public class FileTxnSnapLog {
return this.snapLog.getLastSnapshotInfo();
}
+ /**
+ * whether to force the write of an initial snapshot after a leader election,
+ * to address ZOOKEEPER-3781 after upgrading from Zookeeper 3.4.x.
+ * @return true if an initial snapshot should be written even if not otherwise required, false otherwise.
+ */
+ public boolean shouldForceWriteInitialSnapshotAfterLeaderElection() {
+ return trustEmptySnapshot && getLastSnapshotInfo() == null;
+ }
+
/**
* this function restores the server
* database after reading from the
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
index d281acf2c..44205e83e 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
@@ -550,7 +550,13 @@ public class Learner {
if (qp.getType() == Leader.DIFF) {
LOG.info("Getting a diff from the leader 0x{}", Long.toHexString(qp.getZxid()));
self.setSyncMode(QuorumPeer.SyncMode.DIFF);
- snapshotNeeded = false;
+ if (zk.shouldForceWriteInitialSnapshotAfterLeaderElection()) {
+ LOG.info("Forcing a snapshot write as part of upgrading from an older Zookeeper. This should only happen while upgrading.");
+ snapshotNeeded = true;
+ syncSnapshot = true;
+ } else {
+ snapshotNeeded = false;
+ }
} else if (qp.getType() == Leader.SNAP) {
self.setSyncMode(QuorumPeer.SyncMode.SNAP);
LOG.info("Getting a snapshot from leader 0x{}", Long.toHexString(qp.getZxid()));
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/EmptiedSnapshotRecoveryTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/EmptiedSnapshotRecoveryTest.java
index da50a9c17..3bc164432 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/EmptiedSnapshotRecoveryTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/EmptiedSnapshotRecoveryTest.java
@@ -19,11 +19,13 @@
package org.apache.zookeeper.test;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import java.io.File;
import java.io.IOException;
import java.io.PrintWriter;
+import java.nio.file.Files;
import java.util.List;
import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.PortAssignment;
@@ -143,6 +145,54 @@ public class EmptiedSnapshotRecoveryTest extends ZKTestCase implements Watcher {
runTest(false, true);
}
+ @Test
+ public void testRestoreWithTrustedEmptySnapFilesWhenFollowing() throws Exception {
+ QuorumUtil qu = new QuorumUtil(1);
+ try {
+ qu.startAll();
+ String connString = qu.getConnectionStringForServer(1);
+ try (ZooKeeper zk = new ZooKeeper(connString, CONNECTION_TIMEOUT, this)) {
+ for (int i = 0; i < N_TRANSACTIONS; i++) {
+ zk.create("/node-" + i, new byte[0], Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+ }
+ }
+ int leaderIndex = qu.getLeaderServer();
+ //Shut down the cluster and delete the snapshots from the followers
+ for (int i = 1; i <= qu.ALL; i++) {
+ qu.shutdown(i);
+ if (i != leaderIndex) {
+ FileTxnSnapLog txnLogFactory = qu.getPeer(i).peer.getTxnFactory();
+ List<File> snapshots = txnLogFactory.findNRecentSnapshots(10);
+ assertTrue("We have a snapshot to corrupt", snapshots.size() > 0);
+ for (File file : snapshots) {
+ Files.delete(file.toPath());
+ }
+ assertEquals(txnLogFactory.findNRecentSnapshots(10).size(), 0);
+ }
+ }
+ //Start while trusting empty snapshots, verify that the followers save snapshots
+ System.setProperty(FileTxnSnapLog.ZOOKEEPER_SNAPSHOT_TRUST_EMPTY, "true");
+ qu.start(leaderIndex);
+ for (int i = 1; i <= qu.ALL; i++) {
+ if (i != leaderIndex) {
+ qu.restart(i);
+ FileTxnSnapLog txnLogFactory = qu.getPeer(i).peer.getTxnFactory();
+ List<File> snapshots = txnLogFactory.findNRecentSnapshots(10);
+ assertTrue("A snapshot should have been created on follower " + i, snapshots.size() > 0);
+ }
+ }
+ //Check that the created nodes are still there
+ try (ZooKeeper zk = new ZooKeeper(connString, CONNECTION_TIMEOUT, this)) {
+ for (int i = 0; i < N_TRANSACTIONS; i++) {
+ assertNotNull(zk.exists("/node-" + i, false));
+ }
+ }
+ } finally {
+ System.clearProperty(FileTxnSnapLog.ZOOKEEPER_SNAPSHOT_TRUST_EMPTY);
+ qu.tearDown();
+ }
+ }
+
public void process(WatchedEvent event) {
// do nothing
}
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/QuorumUtil.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/QuorumUtil.java
index f79358519..d5e0d6692 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/QuorumUtil.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/QuorumUtil.java
@@ -251,22 +251,22 @@ public class QuorumUtil {
public void shutdown(int id) {
QuorumPeer qp = getPeer(id).peer;
try {
- LOG.info("Shutting down quorum peer {}", qp.getName());
+ LOG.info("Shutting down quorum peer {} with id {}", qp.getName(), id);
qp.shutdown();
Election e = qp.getElectionAlg();
if (e != null) {
- LOG.info("Shutting down leader election {}", qp.getName());
+ LOG.info("Shutting down leader election {} with id {}", qp.getName(), id);
e.shutdown();
} else {
- LOG.info("No election available to shutdown {}", qp.getName());
+ LOG.info("No election available to shutdown {} with id {}", qp.getName(), id);
}
- LOG.info("Waiting for {} to exit thread", qp.getName());
+ LOG.info("Waiting for {} with id {} to exit thread", qp.getName(), id);
qp.join(30000);
if (qp.isAlive()) {
- fail("QP failed to shutdown in 30 seconds: " + qp.getName());
+ fail("QP failed to shutdown in 30 seconds: " + qp.getName() + " " + id);
}
} catch (InterruptedException e) {
- LOG.debug("QP interrupted: {}", qp.getName(), e);
+ LOG.debug("QP interrupted: {} {}", qp.getName(), id, e);
}
}