Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need this change? In general, when rolling back a procedure, finally we will arrive the MERGE_TABLE_REGIONS_CLOSE_REGIONS state and reopen the parent regions?

case MERGE_TABLE_REGIONS_CLOSE_REGIONS:
rollbackCloseRegionsForMerge(env);
// No-op. The reopen TransitRegionStateProcedures for the two parents were already
// submitted by the rollback of MERGE_TABLE_REGIONS_CHECK_CLOSED_REGIONS above;
// submitting a second batch here would race the first. Mirrors
// SplitTableRegionProcedure's rollback of SPLIT_TABLE_REGION_CLOSE_PARENT_REGION.
break;
case MERGE_TABLE_REGIONS_PRE_MERGE_OPERATION:
postRollBackMergeRegions(env);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import java.util.ArrayList;
import java.util.List;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.HBaseTestingUtil;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.MetaTableAccessor;
Expand All @@ -38,6 +40,7 @@
import org.apache.hadoop.hbase.client.Table;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.master.RegionState;
import org.apache.hadoop.hbase.master.procedure.MasterProcedureConstants;
import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv;
import org.apache.hadoop.hbase.master.procedure.MasterProcedureTestingUtility;
Expand All @@ -51,7 +54,10 @@
import org.apache.hadoop.hbase.testclassification.LargeTests;
import org.apache.hadoop.hbase.testclassification.MasterTests;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.util.CommonFSUtils;
import org.apache.hadoop.hbase.util.FSUtils;
import org.apache.hadoop.hbase.util.Threads;
import org.apache.hadoop.hbase.wal.WALSplitUtil;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
Expand Down Expand Up @@ -327,6 +333,69 @@ 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);

UTIL.waitFor(30_000, 500, () -> {
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;
});
}

@Test
public void testMergeWithoutPONR() throws Exception {
final TableName tableName = TableName.valueOf("testMergeWithoutPONR");
Expand Down