Apache9 commented on code in PR #8745:
URL: https://github.com/apache/hbase/pull/8745#discussion_r4237986028
##########
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();
Review Comment:
Just change the method name?
##########
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
Review Comment:
Just change the method name?
##########
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
Review Comment:
So why the above code could fail?
##########
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
Review Comment:
Ditto.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
