From f4fbba0e1c6a01ff563886b1ebdb2396122c7a7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20Luis=20L=C3=B3pez=20L=C3=B3pez?= Date: Tue, 6 Oct 2026 20:38:29 +0200 Subject: [PATCH 1/3] HBASE-30466 TestSyncReplication* and replication/WAL replay tests do not shut down mini clusters after a failed start or a hidden @AfterAll SyncReplicationTestBaseNoBeforeAll skipped shutdownMiniCluster() when the HBase part of a cluster was absent, which is exactly the state a failed startMiniCluster() leaves behind: DFS still up and the util still marked running. A surefire rerun in the same JVM then fails with "A mini-cluster is already running" (seen on PR 8743, large-wave-3). Teardown is now done in finally blocks so a failing admin call cannot skip it either. TestClaimReplicationQueue, TestRemovePeerProcedureWaitForSCP and TestAsyncWALReplay declare a tearDownAfterClass() that hides the base class one, so their mini clusters were never shut down. They now call the base teardown in finally. Co-Authored-By: Claude Opus 5.5 --- .../regionserver/wal/TestAsyncWALReplay.java | 9 +++- .../SyncReplicationTestBaseNoBeforeAll.java | 43 ++++++++++++------- .../TestClaimReplicationQueue.java | 9 +++- .../TestRemovePeerProcedureWaitForSCP.java | 7 ++- 4 files changed, 47 insertions(+), 21 deletions(-) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncWALReplay.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncWALReplay.java index a1b64817fecf..a86249f5ff84 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncWALReplay.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncWALReplay.java @@ -66,8 +66,13 @@ public static void setUpBeforeClass() throws Exception { @AfterAll public static void tearDownAfterClass() throws Exception { - if (GROUP != null) { - GROUP.shutdownGracefully(); + try { + if (GROUP != null) { + GROUP.shutdownGracefully(); + } + } finally { + // this method hides the base one, so call it to shut down the mini cluster + AbstractTestWALReplay.tearDownAfterClass(); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/SyncReplicationTestBaseNoBeforeAll.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/SyncReplicationTestBaseNoBeforeAll.java index 06d46c37e1d2..b38c09f77a22 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/SyncReplicationTestBaseNoBeforeAll.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/SyncReplicationTestBaseNoBeforeAll.java @@ -136,28 +136,39 @@ protected static void startClusters() throws Exception { } private static void shutdown(HBaseTestingUtil util) throws Exception { - if (util.getHBaseCluster() == null) { - return; - } - Admin admin = util.getAdmin(); - if (!admin.listReplicationPeers(Pattern.compile(PEER_ID)).isEmpty()) { - if ( - admin.getReplicationPeerSyncReplicationState(PEER_ID) - != SyncReplicationState.DOWNGRADE_ACTIVE - ) { - admin.transitReplicationPeerSyncReplicationState(PEER_ID, - SyncReplicationState.DOWNGRADE_ACTIVE); + try { + if (util.getHBaseCluster() == null) { + return; } - admin.removeReplicationPeer(PEER_ID); + Admin admin = util.getAdmin(); + if (!admin.listReplicationPeers(Pattern.compile(PEER_ID)).isEmpty()) { + if ( + admin.getReplicationPeerSyncReplicationState(PEER_ID) + != SyncReplicationState.DOWNGRADE_ACTIVE + ) { + admin.transitReplicationPeerSyncReplicationState(PEER_ID, + SyncReplicationState.DOWNGRADE_ACTIVE); + } + admin.removeReplicationPeer(PEER_ID); + } + } finally { + // Always, even if the HBase part never came up: a failed startMiniCluster leaves DFS running + // and the util marked as running, so a surefire rerun in this JVM would fail to start. + util.shutdownMiniCluster(); } - util.shutdownMiniCluster(); } @AfterAll public static void tearDown() throws Exception { - shutdown(UTIL1); - shutdown(UTIL2); - ZK_UTIL.shutdownMiniZKCluster(); + try { + shutdown(UTIL1); + } finally { + try { + shutdown(UTIL2); + } finally { + ZK_UTIL.shutdownMiniZKCluster(); + } + } } protected final void write(HBaseTestingUtil util, int start, int end) throws IOException { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestClaimReplicationQueue.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestClaimReplicationQueue.java index b7f6558d55b7..4fd78b503c90 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestClaimReplicationQueue.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestClaimReplicationQueue.java @@ -109,8 +109,13 @@ public static void setUpBeforeClass() throws Exception { @AfterAll public static void tearDownAfterClass() throws Exception { - Closeables.close(table3, true); - Closeables.close(table4, true); + try { + Closeables.close(table3, true); + Closeables.close(table4, true); + } finally { + // this method hides the base one, so call it to shut down the mini clusters + TestReplicationBaseNoBeforeAll.tearDownAfterClass(); + } } @BeforeEach diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestRemovePeerProcedureWaitForSCP.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestRemovePeerProcedureWaitForSCP.java index b3bfccafe1f1..709b501fee50 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestRemovePeerProcedureWaitForSCP.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestRemovePeerProcedureWaitForSCP.java @@ -125,7 +125,12 @@ public void removeExtraPeer() throws Exception { @AfterAll public static void tearDownAfterClass() throws Exception { - Closeables.close(table3, true); + try { + Closeables.close(table3, true); + } finally { + // this method hides the base one, so call it to shut down the mini clusters + TestReplicationBaseNoBeforeAll.tearDownAfterClass(); + } } @Test From 6684ac08c27f602214ef62a3bdd2f09078327338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20Luis=20L=C3=B3pez=20L=C3=B3pez?= Date: Sat, 10 Oct 2026 17:17:07 +0200 Subject: [PATCH 2/3] HBASE-30466 Address review: rename hiding @AfterAll methods, simplify sync replication shutdown Rename the subclass @AfterAll methods instead of calling the hidden base teardown explicitly, so JUnit runs both. Also fix TestNamespaceReplication, which hid the base teardown in the same way. Drop the early return inside the try in SyncReplicationTestBaseNoBeforeAll#shutdown. Co-Authored-By: Claude Opus 5.5 --- .../regionserver/wal/TestAsyncWALReplay.java | 11 +++----- .../SyncReplicationTestBaseNoBeforeAll.java | 27 +++++++++---------- .../TestClaimReplicationQueue.java | 11 +++----- .../replication/TestNamespaceReplication.java | 2 +- .../TestRemovePeerProcedureWaitForSCP.java | 9 ++----- 5 files changed, 22 insertions(+), 38 deletions(-) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncWALReplay.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncWALReplay.java index a86249f5ff84..9c14c222465c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncWALReplay.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncWALReplay.java @@ -65,14 +65,9 @@ public static void setUpBeforeClass() throws Exception { } @AfterAll - public static void tearDownAfterClass() throws Exception { - try { - if (GROUP != null) { - GROUP.shutdownGracefully(); - } - } finally { - // this method hides the base one, so call it to shut down the mini cluster - AbstractTestWALReplay.tearDownAfterClass(); + public static void tearDownAfterAll() throws Exception { + if (GROUP != null) { + GROUP.shutdownGracefully(); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/SyncReplicationTestBaseNoBeforeAll.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/SyncReplicationTestBaseNoBeforeAll.java index b38c09f77a22..172f53ad52a1 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/SyncReplicationTestBaseNoBeforeAll.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/SyncReplicationTestBaseNoBeforeAll.java @@ -137,23 +137,22 @@ protected static void startClusters() throws Exception { private static void shutdown(HBaseTestingUtil util) throws Exception { try { - if (util.getHBaseCluster() == null) { - return; - } - Admin admin = util.getAdmin(); - if (!admin.listReplicationPeers(Pattern.compile(PEER_ID)).isEmpty()) { - if ( - admin.getReplicationPeerSyncReplicationState(PEER_ID) - != SyncReplicationState.DOWNGRADE_ACTIVE - ) { - admin.transitReplicationPeerSyncReplicationState(PEER_ID, - SyncReplicationState.DOWNGRADE_ACTIVE); + if (util.getHBaseCluster() != null) { + Admin admin = util.getAdmin(); + if (!admin.listReplicationPeers(Pattern.compile(PEER_ID)).isEmpty()) { + if ( + admin.getReplicationPeerSyncReplicationState(PEER_ID) + != SyncReplicationState.DOWNGRADE_ACTIVE + ) { + admin.transitReplicationPeerSyncReplicationState(PEER_ID, + SyncReplicationState.DOWNGRADE_ACTIVE); + } + admin.removeReplicationPeer(PEER_ID); } - admin.removeReplicationPeer(PEER_ID); } } finally { - // Always, even if the HBase part never came up: a failed startMiniCluster leaves DFS running - // and the util marked as running, so a surefire rerun in this JVM would fail to start. + // Always, even if the cluster failed to start or the peer cleanup above failed on a broken + // cluster: otherwise DFS is left running and a surefire rerun in this JVM fails to start. util.shutdownMiniCluster(); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestClaimReplicationQueue.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestClaimReplicationQueue.java index 4fd78b503c90..336eb95f1faa 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestClaimReplicationQueue.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestClaimReplicationQueue.java @@ -108,14 +108,9 @@ public static void setUpBeforeClass() throws Exception { } @AfterAll - public static void tearDownAfterClass() throws Exception { - try { - Closeables.close(table3, true); - Closeables.close(table4, true); - } finally { - // this method hides the base one, so call it to shut down the mini clusters - TestReplicationBaseNoBeforeAll.tearDownAfterClass(); - } + public static void tearDownAfterAll() throws Exception { + Closeables.close(table3, true); + Closeables.close(table4, true); } @BeforeEach diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestNamespaceReplication.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestNamespaceReplication.java index a2a2bc9015cc..12e1a1a97432 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestNamespaceReplication.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestNamespaceReplication.java @@ -124,7 +124,7 @@ public static void setUpBeforeClass() throws Exception { } @AfterAll - public static void tearDownAfterClass() throws Exception { + public static void tearDownAfterAll() throws Exception { admin1.disableTable(tabAName); admin1.deleteTable(tabAName); admin1.disableTable(tabBName); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestRemovePeerProcedureWaitForSCP.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestRemovePeerProcedureWaitForSCP.java index 709b501fee50..9037d4e9ec8f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestRemovePeerProcedureWaitForSCP.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestRemovePeerProcedureWaitForSCP.java @@ -124,13 +124,8 @@ public void removeExtraPeer() throws Exception { } @AfterAll - public static void tearDownAfterClass() throws Exception { - try { - Closeables.close(table3, true); - } finally { - // this method hides the base one, so call it to shut down the mini clusters - TestReplicationBaseNoBeforeAll.tearDownAfterClass(); - } + public static void tearDownAfterAll() throws Exception { + Closeables.close(table3, true); } @Test From dd3e14199b48c941d7a6c576ed16c4fc3754ffc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20Luis=20L=C3=B3pez=20L=C3=B3pez?= Date: Fri, 16 Oct 2026 07:45:48 +0200 Subject: [PATCH 3/3] HBASE-30466 Rename the remaining hiding @AfterAll methods TestAsyncFSWAL, TestAsyncProtobufLog and TestVerifyReplicationAdjunct also hid the base class tearDownAfterClass(), so JUnit 5 skipped it and their mini clusters were never shut down. Co-Authored-By: Claude Opus 5.5 --- .../hadoop/hbase/replication/TestVerifyReplicationAdjunct.java | 2 +- .../apache/hadoop/hbase/regionserver/wal/TestAsyncFSWAL.java | 2 +- .../hadoop/hbase/regionserver/wal/TestAsyncProtobufLog.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/replication/TestVerifyReplicationAdjunct.java b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/replication/TestVerifyReplicationAdjunct.java index 21528de6dc63..8049cba7aaa0 100644 --- a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/replication/TestVerifyReplicationAdjunct.java +++ b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/replication/TestVerifyReplicationAdjunct.java @@ -342,7 +342,7 @@ public void testVerifyReplicationWithSnapshotSupport() throws Exception { } @AfterAll - public static void tearDownAfterClass() throws Exception { + public static void tearDownAfterAll() throws Exception { htable3.close(); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncFSWAL.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncFSWAL.java index 5dd5e3b26c89..33dd3e00d6f3 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncFSWAL.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncFSWAL.java @@ -88,7 +88,7 @@ public static void setUpBeforeClass() throws Exception { } @AfterAll - public static void tearDownAfterClass() throws Exception { + public static void tearDownAfterAll() throws Exception { if (GROUP != null) { GROUP.shutdownGracefully(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncProtobufLog.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncProtobufLog.java index 0e89201d4374..7de41b104ff3 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncProtobufLog.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncProtobufLog.java @@ -47,7 +47,7 @@ public static void setUpBeforeClass() throws Exception { } @AfterAll - public static void tearDownAfterClass() throws Exception { + public static void tearDownAfterAll() throws Exception { if (EVENT_LOOP_GROUP != null) { EVENT_LOOP_GROUP.shutdownGracefully().syncUninterruptibly(); }