Apache9 commented on code in PR #8583:
URL: https://github.com/apache/hbase/pull/8583#discussion_r3903378239
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java:
##########
@@ -327,6 +333,80 @@ public void testRollbackAndDoubleExecution() throws
Exception {
assertEquals(initialRegionCount, regions.size());
}
+ /**
+ * HBASE-30334 repro. Plant a stale recovered.edits file on a parent region
so that
+ * MERGE_TABLE_REGIONS_CHECK_CLOSED_REGIONS throws (the exact failure from
the 49-min
+ * stuck-RIT incident). After rollback the parents MUST be OPEN. If they
stay stuck
+ * (e.g. CLOSED / MERGING), we've reproduced the incident locally and the
rollback path
+ * as-is is broken.
+ */
+ @Test
+ public void testRollbackReopensParentsAfterCheckClosedRegionsFailure()
throws Exception {
+ final TableName tableName = TableName.valueOf(testMethodName);
+ UTIL.createTable(tableName, new byte[][] { HConstants.CATALOG_FAMILY },
+ new byte[][] { new byte[] { 'b' } });
+ UTIL.waitUntilAllRegionsAssigned(tableName);
+
+ List<RegionInfo> ris =
MetaTableAccessor.getTableRegions(UTIL.getConnection(), tableName);
+ assertEquals(2, ris.size());
+ RegionInfo[] regionsToMerge = new RegionInfo[] { ris.get(0), ris.get(1) };
+
+ Configuration conf = UTIL.getConfiguration();
+ Path regionDir =
+ FSUtils.getRegionDirFromRootDir(CommonFSUtils.getRootDir(conf),
regionsToMerge[0]);
+ Path recoveredEditsDir =
WALSplitUtil.getRegionDirRecoveredEditsDir(regionDir);
+ FileSystem fs = CommonFSUtils.getRootDirFileSystem(conf);
+ fs.mkdirs(recoveredEditsDir);
+ Path staleFile = new Path(recoveredEditsDir, "0000000000000000001");
+ fs.createNewFile(staleFile);
+ assertTrue(WALSplitUtil.hasRecoveredEdits(conf, regionsToMerge[0]),
+ "stale recovered.edits file must be visible");
+
+ AssignmentManager am =
UTIL.getHBaseCluster().getMaster().getAssignmentManager();
+ LOG.info("HBASE-30334-DEBUG pre-merge: {} state={} / {} state={}",
+ regionsToMerge[0].getEncodedName(),
+ am.getRegionStates().getRegionStateNode(regionsToMerge[0]).getState(),
+ regionsToMerge[1].getEncodedName(),
+ am.getRegionStates().getRegionStateNode(regionsToMerge[1]).getState());
+
+ final ProcedureExecutor<MasterProcedureEnv> procExec =
getMasterProcedureExecutor();
+ MergeTableRegionsProcedure proc =
+ new MergeTableRegionsProcedure(procExec.getEnvironment(),
regionsToMerge, true);
+ long procId = procExec.submitProcedure(proc);
+ ProcedureTestingUtility.waitProcedure(procExec, procId);
+
+ RegionState.State s0Post =
+ am.getRegionStates().getRegionStateNode(regionsToMerge[0]).getState();
+ RegionState.State s1Post =
+ am.getRegionStates().getRegionStateNode(regionsToMerge[1]).getState();
+ LOG.info("HBASE-30334-DEBUG post-rollback: {} state={} / {} state={}",
+ regionsToMerge[0].getEncodedName(), s0Post,
+ regionsToMerge[1].getEncodedName(), s1Post);
+
+ ProcedureTestingUtility.assertProcFailed(procExec, procId);
+
+ fs.delete(staleFile, false);
+
+ long elapsed = org.apache.hadoop.hbase.Waiter.waitFor(conf, 30_000, 500,
false,
Review Comment:
Just use UTIL.waitFor?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java:
##########
@@ -284,9 +284,13 @@ protected void rollbackState(final MasterProcedureEnv env,
final MergeTableRegio
cleanupMergedRegion(env);
break;
case MERGE_TABLE_REGIONS_CHECK_CLOSED_REGIONS:
+ rollbackCloseRegionsForMerge(env);
break;
case MERGE_TABLE_REGIONS_CLOSE_REGIONS:
- rollbackCloseRegionsForMerge(env);
+ // If it rolls back with state MERGE_TABLE_REGIONS_CLOSE_REGIONS, no
need to call
+ // rollbackCloseRegionsForMerge(), otherwise, it will result in
duplicate
+ // TransitRegionStateProcedures for parents that are already
OPENING. Mirrors
Review Comment:
Already opened?
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java:
##########
@@ -327,6 +333,80 @@ public void testRollbackAndDoubleExecution() throws
Exception {
assertEquals(initialRegionCount, regions.size());
}
+ /**
+ * HBASE-30334 repro. Plant a stale recovered.edits file on a parent region
so that
+ * MERGE_TABLE_REGIONS_CHECK_CLOSED_REGIONS throws (the exact failure from
the 49-min
+ * stuck-RIT incident). After rollback the parents MUST be OPEN. If they
stay stuck
+ * (e.g. CLOSED / MERGING), we've reproduced the incident locally and the
rollback path
+ * as-is is broken.
+ */
+ @Test
+ public void testRollbackReopensParentsAfterCheckClosedRegionsFailure()
throws Exception {
+ final TableName tableName = TableName.valueOf(testMethodName);
+ UTIL.createTable(tableName, new byte[][] { HConstants.CATALOG_FAMILY },
+ new byte[][] { new byte[] { 'b' } });
+ UTIL.waitUntilAllRegionsAssigned(tableName);
+
+ List<RegionInfo> ris =
MetaTableAccessor.getTableRegions(UTIL.getConnection(), tableName);
+ assertEquals(2, ris.size());
+ RegionInfo[] regionsToMerge = new RegionInfo[] { ris.get(0), ris.get(1) };
+
+ Configuration conf = UTIL.getConfiguration();
+ Path regionDir =
+ FSUtils.getRegionDirFromRootDir(CommonFSUtils.getRootDir(conf),
regionsToMerge[0]);
+ Path recoveredEditsDir =
WALSplitUtil.getRegionDirRecoveredEditsDir(regionDir);
+ FileSystem fs = CommonFSUtils.getRootDirFileSystem(conf);
+ fs.mkdirs(recoveredEditsDir);
+ Path staleFile = new Path(recoveredEditsDir, "0000000000000000001");
+ fs.createNewFile(staleFile);
+ assertTrue(WALSplitUtil.hasRecoveredEdits(conf, regionsToMerge[0]),
+ "stale recovered.edits file must be visible");
+
+ AssignmentManager am =
UTIL.getHBaseCluster().getMaster().getAssignmentManager();
+ LOG.info("HBASE-30334-DEBUG pre-merge: {} state={} / {} state={}",
+ regionsToMerge[0].getEncodedName(),
+ am.getRegionStates().getRegionStateNode(regionsToMerge[0]).getState(),
+ regionsToMerge[1].getEncodedName(),
+ am.getRegionStates().getRegionStateNode(regionsToMerge[1]).getState());
+
+ final ProcedureExecutor<MasterProcedureEnv> procExec =
getMasterProcedureExecutor();
+ MergeTableRegionsProcedure proc =
+ new MergeTableRegionsProcedure(procExec.getEnvironment(),
regionsToMerge, true);
+ long procId = procExec.submitProcedure(proc);
+ ProcedureTestingUtility.waitProcedure(procExec, procId);
+
+ RegionState.State s0Post =
+ am.getRegionStates().getRegionStateNode(regionsToMerge[0]).getState();
+ RegionState.State s1Post =
+ am.getRegionStates().getRegionStateNode(regionsToMerge[1]).getState();
+ LOG.info("HBASE-30334-DEBUG post-rollback: {} state={} / {} state={}",
+ regionsToMerge[0].getEncodedName(), s0Post,
+ regionsToMerge[1].getEncodedName(), s1Post);
+
+ ProcedureTestingUtility.assertProcFailed(procExec, procId);
+
+ fs.delete(staleFile, false);
+
+ long elapsed = org.apache.hadoop.hbase.Waiter.waitFor(conf, 30_000, 500,
false,
+ new org.apache.hadoop.hbase.Waiter.Predicate<Exception>() {
+ @Override
+ public boolean evaluate() {
+ RegionState.State a =
+
am.getRegionStates().getRegionStateNode(regionsToMerge[0]).getState();
+ RegionState.State b =
+
am.getRegionStates().getRegionStateNode(regionsToMerge[1]).getState();
+ return a == RegionState.State.OPEN && b == RegionState.State.OPEN;
+ }
+ });
+
+ assertTrue(elapsed > 0,
Review Comment:
What is the intention for asserting elapsed > 0?
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java:
##########
@@ -327,6 +333,80 @@ public void testRollbackAndDoubleExecution() throws
Exception {
assertEquals(initialRegionCount, regions.size());
}
+ /**
+ * HBASE-30334 repro. Plant a stale recovered.edits file on a parent region
so that
+ * MERGE_TABLE_REGIONS_CHECK_CLOSED_REGIONS throws (the exact failure from
the 49-min
+ * stuck-RIT incident). After rollback the parents MUST be OPEN. If they
stay stuck
+ * (e.g. CLOSED / MERGING), we've reproduced the incident locally and the
rollback path
+ * as-is is broken.
+ */
+ @Test
+ public void testRollbackReopensParentsAfterCheckClosedRegionsFailure()
throws Exception {
+ final TableName tableName = TableName.valueOf(testMethodName);
+ UTIL.createTable(tableName, new byte[][] { HConstants.CATALOG_FAMILY },
+ new byte[][] { new byte[] { 'b' } });
+ UTIL.waitUntilAllRegionsAssigned(tableName);
+
+ List<RegionInfo> ris =
MetaTableAccessor.getTableRegions(UTIL.getConnection(), tableName);
+ assertEquals(2, ris.size());
+ RegionInfo[] regionsToMerge = new RegionInfo[] { ris.get(0), ris.get(1) };
+
+ Configuration conf = UTIL.getConfiguration();
+ Path regionDir =
+ FSUtils.getRegionDirFromRootDir(CommonFSUtils.getRootDir(conf),
regionsToMerge[0]);
+ Path recoveredEditsDir =
WALSplitUtil.getRegionDirRecoveredEditsDir(regionDir);
+ FileSystem fs = CommonFSUtils.getRootDirFileSystem(conf);
+ fs.mkdirs(recoveredEditsDir);
+ Path staleFile = new Path(recoveredEditsDir, "0000000000000000001");
+ fs.createNewFile(staleFile);
+ assertTrue(WALSplitUtil.hasRecoveredEdits(conf, regionsToMerge[0]),
+ "stale recovered.edits file must be visible");
+
+ AssignmentManager am =
UTIL.getHBaseCluster().getMaster().getAssignmentManager();
+ LOG.info("HBASE-30334-DEBUG pre-merge: {} state={} / {} state={}",
+ regionsToMerge[0].getEncodedName(),
+ am.getRegionStates().getRegionStateNode(regionsToMerge[0]).getState(),
+ regionsToMerge[1].getEncodedName(),
+ am.getRegionStates().getRegionStateNode(regionsToMerge[1]).getState());
+
+ final ProcedureExecutor<MasterProcedureEnv> procExec =
getMasterProcedureExecutor();
+ MergeTableRegionsProcedure proc =
+ new MergeTableRegionsProcedure(procExec.getEnvironment(),
regionsToMerge, true);
+ long procId = procExec.submitProcedure(proc);
+ ProcedureTestingUtility.waitProcedure(procExec, procId);
+
+ RegionState.State s0Post =
+ am.getRegionStates().getRegionStateNode(regionsToMerge[0]).getState();
+ RegionState.State s1Post =
+ am.getRegionStates().getRegionStateNode(regionsToMerge[1]).getState();
+ LOG.info("HBASE-30334-DEBUG post-rollback: {} state={} / {} state={}",
+ regionsToMerge[0].getEncodedName(), s0Post,
+ regionsToMerge[1].getEncodedName(), s1Post);
+
+ ProcedureTestingUtility.assertProcFailed(procExec, procId);
+
+ fs.delete(staleFile, false);
+
+ long elapsed = org.apache.hadoop.hbase.Waiter.waitFor(conf, 30_000, 500,
false,
+ new org.apache.hadoop.hbase.Waiter.Predicate<Exception>() {
Review Comment:
Use lambda?
--
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]
