From c1c4d8748b2649814322fb47c69042168e308f27 Mon Sep 17 00:00:00 2001 From: Sirius Date: Thu, 6 Jul 2023 20:59:05 +0800 Subject: [PATCH 1/2] ZOOKEEPER-4712: Follower.shutdown() and Observer.shutdown() do not correctly shutdown the syncProcessor, which may lead to data inconsistency --- .../zookeeper/server/ZooKeeperServer.java | 25 ++++++++++++++++--- .../server/quorum/LearnerZooKeeperServer.java | 14 +++++++++++ 2 files changed, 35 insertions(+), 4 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 f460b2d5bb4..e67ef66e371 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 @@ -933,12 +933,23 @@ public void shutdown() { * @param fullyShutDown true if another server using the same database will not replace this one in the same process */ public synchronized void shutdown(boolean fullyShutDown) { + if (shutdownZKServer(fullyShutDown)) { + updateZKDatabase(fullyShutDown); + } + } + + /** + * Shut down the server instance + * @param fullyShutDown true if another server using the same database will not replace this one in the same process + * @return true if the server is successfully shutdown, false if the server cannot be shutdown. + */ + public synchronized boolean shutdownZKServer(boolean fullyShutDown) { if (!canShutdown()) { if (fullyShutDown && zkDb != null) { zkDb.clear(); } LOG.debug("ZooKeeper server is not running, so not proceeding to shutdown!"); - return; + return false; } LOG.info("shutting down"); @@ -965,7 +976,16 @@ public synchronized void shutdown(boolean fullyShutDown) { if (jvmPauseMonitor != null) { jvmPauseMonitor.serviceStop(); } + requestPathMetricsCollector.shutdown(); + unregisterJMX(); + return true; + } + /** + * Update zk database during shutdown + * @param fullyShutDown true if another server using the same database will not replace this one in the same process + */ + public synchronized void updateZKDatabase(boolean fullyShutDown) { if (zkDb != null) { if (fullyShutDown) { zkDb.clear(); @@ -984,9 +1004,6 @@ public synchronized void shutdown(boolean fullyShutDown) { } } } - - requestPathMetricsCollector.shutdown(); - unregisterJMX(); } protected void unregisterJMX() { diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java index efd2e376208..440a1917f95 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LearnerZooKeeperServer.java @@ -172,4 +172,18 @@ public synchronized void shutdown() { } } + @Override + public void shutdown(boolean fullyShutDown) { + if (shutdownZKServer(fullyShutDown)) { + try { + if (syncProcessor != null) { + syncProcessor.shutdown(); + } + } catch (Exception e) { + LOG.warn("Ignoring unexpected exception in syncprocessor shutdown", e); + } + updateZKDatabase(fullyShutDown); + } + } + } From 7a101128c82f2d33cc26f4a0ad8e6c929f5c3bdb Mon Sep 17 00:00:00 2001 From: Sirius Date: Fri, 7 Jul 2023 01:04:31 +0800 Subject: [PATCH 2/2] ZOOKEEPER-4712: Follower.shutdown() and Observer.shutdown() do not correctly shutdown the syncProcessor, which may lead to data inconsistency --- .../java/org/apache/zookeeper/server/ZooKeeperServer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 e67ef66e371..43d06816661 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 @@ -943,7 +943,7 @@ public synchronized void shutdown(boolean fullyShutDown) { * @param fullyShutDown true if another server using the same database will not replace this one in the same process * @return true if the server is successfully shutdown, false if the server cannot be shutdown. */ - public synchronized boolean shutdownZKServer(boolean fullyShutDown) { + protected synchronized boolean shutdownZKServer(boolean fullyShutDown) { if (!canShutdown()) { if (fullyShutDown && zkDb != null) { zkDb.clear(); @@ -985,7 +985,7 @@ public synchronized boolean shutdownZKServer(boolean fullyShutDown) { * Update zk database during shutdown * @param fullyShutDown true if another server using the same database will not replace this one in the same process */ - public synchronized void updateZKDatabase(boolean fullyShutDown) { + protected synchronized void updateZKDatabase(boolean fullyShutDown) { if (zkDb != null) { if (fullyShutDown) { zkDb.clear();