From 4a4d108fe71670142acca8d08f125828f8c0b256 Mon Sep 17 00:00:00 2001 From: feynmanlin <315157973@qq.com> Date: Sun, 25 Aug 2024 21:08:38 +0800 Subject: [PATCH 1/7] [fix]Decommission command throws KeeperErrorCode exception when autoRecovery is disabled --- .../bookkeeper/meta/ZkLedgerAuditorManager.java | 2 +- .../meta/ZkLedgerUnderreplicationManager.java | 5 +++++ .../bookkeeper/bookie/ClusterInfoCommandTest.java | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerAuditorManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerAuditorManager.java index 4f52245514c..7633e3aa404 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerAuditorManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerAuditorManager.java @@ -59,7 +59,7 @@ public class ZkLedgerAuditorManager implements LedgerAuditorManager { private String myVote; - private static final String ELECTION_ZNODE = "auditorelection"; + public static final String ELECTION_ZNODE = "auditorelection"; // Represents the index of the auditor node private static final int AUDITOR_INDEX = 0; diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java index 4118e191319..e5689df219a 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java @@ -19,6 +19,7 @@ package org.apache.bookkeeper.meta; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.apache.bookkeeper.meta.ZkLedgerAuditorManager.ELECTION_ZNODE; import com.google.common.base.Joiner; import com.google.common.util.concurrent.RateLimiter; @@ -737,6 +738,10 @@ public boolean isLedgerReplicationEnabled() LOG.debug("isLedgerReplicationEnabled()"); } try { + String electionRoot = basePath + '/' + BookKeeperConstants.UNDER_REPLICATION_NODE + '/' + ELECTION_ZNODE; + if (zkc.exists(electionRoot, false) == null) { + return false; + } return null == zkc.exists(basePath + '/' + BookKeeperConstants.DISABLE_NODE, false); } catch (KeeperException ke) { diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java index 2e1bb55ce1b..5b6913b63c7 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java @@ -23,8 +23,11 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import org.apache.bookkeeper.client.BookKeeperAdmin; import org.apache.bookkeeper.conf.ServerConfiguration; +import org.apache.bookkeeper.replication.ReplicationException; import org.apache.bookkeeper.test.BookKeeperClusterTestCase; import org.apache.bookkeeper.tools.cli.commands.bookies.ClusterInfoCommand; import org.apache.bookkeeper.tools.framework.CliFlags; @@ -60,4 +63,16 @@ public void testClusterInfo() throws Exception { assertTrue(info.isLedgerReplicationEnabled()); } + @Test + public void testRecoveryDisabled() throws Exception { + BookKeeperAdmin bookKeeperAdmin = new BookKeeperAdmin(super.bkc); + try { + bookKeeperAdmin.triggerAudit(); + fail("should failed"); + } catch (Exception e) { + assertTrue(e instanceof ReplicationException.UnavailableException); + assertTrue(e.getMessage().contains("Autorecovery is disabled. So giving up")); + } + } + } From 4cfc8607aa8bee92912a46eb9cd6078ae0cd1a23 Mon Sep 17 00:00:00 2001 From: feynmanlin <315157973@qq.com> Date: Sun, 25 Aug 2024 22:45:01 +0800 Subject: [PATCH 2/7] [fix]Decommission command throws KeeperErrorCode exception when autoRecovery is disabled --- .../org/apache/bookkeeper/client/BookKeeperAdmin.java | 11 ++++++++++- .../bookkeeper/meta/ZkLedgerAuditorManager.java | 2 +- .../meta/ZkLedgerUnderreplicationManager.java | 5 ----- .../bookkeeper/bookie/ClusterInfoCommandTest.java | 5 ++--- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java index 371c2145323..82d2bd8f789 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java @@ -1553,7 +1553,16 @@ public void triggerAudit() throw new UnavailableException("Autorecovery is disabled. So giving up!"); } - BookieId auditorId = getLedgerAuditorManager().getCurrentAuditor(); + BookieId auditorId = null; + try { + auditorId = getLedgerAuditorManager().getCurrentAuditor(); + } catch (IOException e) { + if (e.getCause() instanceof KeeperException.NoNodeException) { + LOG.error("Can not find node for {}", e.getCause().getMessage()); + throw new UnavailableException("Autorecovery is disabled. So giving up!"); + } + throw e; + } if (auditorId == null) { LOG.error("No auditor elected, though Autorecovery is enabled. So giving up."); throw new UnavailableException("No auditor elected, though Autorecovery is enabled. So giving up."); diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerAuditorManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerAuditorManager.java index 7633e3aa404..4f52245514c 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerAuditorManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerAuditorManager.java @@ -59,7 +59,7 @@ public class ZkLedgerAuditorManager implements LedgerAuditorManager { private String myVote; - public static final String ELECTION_ZNODE = "auditorelection"; + private static final String ELECTION_ZNODE = "auditorelection"; // Represents the index of the auditor node private static final int AUDITOR_INDEX = 0; diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java index e5689df219a..4118e191319 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java @@ -19,7 +19,6 @@ package org.apache.bookkeeper.meta; import static java.nio.charset.StandardCharsets.UTF_8; -import static org.apache.bookkeeper.meta.ZkLedgerAuditorManager.ELECTION_ZNODE; import com.google.common.base.Joiner; import com.google.common.util.concurrent.RateLimiter; @@ -738,10 +737,6 @@ public boolean isLedgerReplicationEnabled() LOG.debug("isLedgerReplicationEnabled()"); } try { - String electionRoot = basePath + '/' + BookKeeperConstants.UNDER_REPLICATION_NODE + '/' + ELECTION_ZNODE; - if (zkc.exists(electionRoot, false) == null) { - return false; - } return null == zkc.exists(basePath + '/' + BookKeeperConstants.DISABLE_NODE, false); } catch (KeeperException ke) { diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java index 5b6913b63c7..ae60b698a96 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java @@ -64,9 +64,8 @@ public void testClusterInfo() throws Exception { } @Test - public void testRecoveryDisabled() throws Exception { - BookKeeperAdmin bookKeeperAdmin = new BookKeeperAdmin(super.bkc); - try { + public void testRecoveryDisabled() { + try (BookKeeperAdmin bookKeeperAdmin = new BookKeeperAdmin(super.bkc)) { bookKeeperAdmin.triggerAudit(); fail("should failed"); } catch (Exception e) { From b3f8b7ca9d61fd60e524869b3390c97f180d14a9 Mon Sep 17 00:00:00 2001 From: feynmanlin <315157973@qq.com> Date: Thu, 29 Aug 2024 22:05:27 +0800 Subject: [PATCH 3/7] Update bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java Co-authored-by: ZhangJian He --- .../main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java index 82d2bd8f789..4ecd08b726c 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java @@ -1559,7 +1559,7 @@ public void triggerAudit() } catch (IOException e) { if (e.getCause() instanceof KeeperException.NoNodeException) { LOG.error("Can not find node for {}", e.getCause().getMessage()); - throw new UnavailableException("Autorecovery is disabled. So giving up!"); + throw new UnavailableException("Autorecovery is disabled due to missing Zookeeper node. Aborting recovery!"); } throw e; } From 1f9a47eadb4753731004d1815cfb1e421418d046 Mon Sep 17 00:00:00 2001 From: feynmanlin <315157973@qq.com> Date: Thu, 29 Aug 2024 22:05:35 +0800 Subject: [PATCH 4/7] Update bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java Co-authored-by: ZhangJian He --- .../main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java index 4ecd08b726c..b25b9c55440 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java @@ -1558,7 +1558,7 @@ public void triggerAudit() auditorId = getLedgerAuditorManager().getCurrentAuditor(); } catch (IOException e) { if (e.getCause() instanceof KeeperException.NoNodeException) { - LOG.error("Can not find node for {}", e.getCause().getMessage()); + LOG.error("Unable to find Zookeeper node: {}", e.getCause().getMessage()); throw new UnavailableException("Autorecovery is disabled due to missing Zookeeper node. Aborting recovery!"); } throw e; From ecc5ee37713c78e8157779eb49405c6a6e1fb5b8 Mon Sep 17 00:00:00 2001 From: feynmanlin <315157973@qq.com> Date: Thu, 29 Aug 2024 22:08:25 +0800 Subject: [PATCH 5/7] Adopt suggestions --- .../org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java index ae60b698a96..b0c88a0d2af 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java @@ -70,7 +70,7 @@ public void testRecoveryDisabled() { fail("should failed"); } catch (Exception e) { assertTrue(e instanceof ReplicationException.UnavailableException); - assertTrue(e.getMessage().contains("Autorecovery is disabled. So giving up")); + assertTrue(e.getMessage().contains("Autorecovery is disabled due to missing Zookeeper node. Aborting recovery")); } } From 14237cb777f1a88cfaf97077261883b89ebbc339 Mon Sep 17 00:00:00 2001 From: feynmanlin <315157973@qq.com> Date: Thu, 29 Aug 2024 22:28:34 +0800 Subject: [PATCH 6/7] fix check style --- .../java/org/apache/bookkeeper/client/BookKeeperAdmin.java | 3 ++- .../org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java index b25b9c55440..85ddc096df1 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java @@ -1559,7 +1559,8 @@ public void triggerAudit() } catch (IOException e) { if (e.getCause() instanceof KeeperException.NoNodeException) { LOG.error("Unable to find Zookeeper node: {}", e.getCause().getMessage()); - throw new UnavailableException("Autorecovery is disabled due to missing Zookeeper node. Aborting recovery!"); + throw new UnavailableException("Autorecovery is disabled due to " + + "missing Zookeeper node. Aborting recovery!"); } throw e; } diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java index b0c88a0d2af..1e833221a17 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java @@ -70,7 +70,8 @@ public void testRecoveryDisabled() { fail("should failed"); } catch (Exception e) { assertTrue(e instanceof ReplicationException.UnavailableException); - assertTrue(e.getMessage().contains("Autorecovery is disabled due to missing Zookeeper node. Aborting recovery")); + assertTrue(e.getMessage().contains("Autorecovery is disabled due to missing Zookeeper node." + + " Aborting recovery")); } } From 6f66bf3ff13a17ee9af6da4dd5340e823e8cff7e Mon Sep 17 00:00:00 2001 From: feynmanlin <315157973@qq.com> Date: Thu, 29 Aug 2024 22:36:57 +0800 Subject: [PATCH 7/7] fix check style --- .../java/org/apache/bookkeeper/client/BookKeeperAdmin.java | 4 ++-- .../org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java index 85ddc096df1..7bc99b2370f 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java @@ -1559,8 +1559,8 @@ public void triggerAudit() } catch (IOException e) { if (e.getCause() instanceof KeeperException.NoNodeException) { LOG.error("Unable to find Zookeeper node: {}", e.getCause().getMessage()); - throw new UnavailableException("Autorecovery is disabled due to " + - "missing Zookeeper node. Aborting recovery!"); + throw new UnavailableException("Autorecovery is disabled due to " + + "missing Zookeeper node. Aborting recovery!"); } throw e; } diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java index 1e833221a17..de6f68ab768 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/ClusterInfoCommandTest.java @@ -70,8 +70,8 @@ public void testRecoveryDisabled() { fail("should failed"); } catch (Exception e) { assertTrue(e instanceof ReplicationException.UnavailableException); - assertTrue(e.getMessage().contains("Autorecovery is disabled due to missing Zookeeper node." + - " Aborting recovery")); + assertTrue(e.getMessage().contains("Autorecovery is disabled due to missing Zookeeper node." + + " Aborting recovery")); } }