Skip to content

HBASE-30353 Prevent split parent regions from being re-opened #8700

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tanyastickles wants to merge 7 commits into
base: master
Choose a base branch
from
Open

HBASE-30353 Prevent split parent regions from being re-opened #8700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bfb2971
HBASE-30353 Prevent split parent regions from being re-opened after m…
Sep 24, 2026
af158e1
updating comments
Sep 24, 2026
ac031eb
adding another comment
Sep 24, 2026
df28eda
cleaning up -- removing comments from discussion
Sep 25, 2026
a25c826
removing more comments
Sep 25, 2026
e07af65
cleaning up tests
Sep 25, 2026
7aec8bc
adding some unit tests
Sep 25, 2026
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
5 changes: 5 additions & 0 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,7 @@ private void preTransitCheck(RegionStateNode regionNode, RegionState.State[] exp
if (!regionNode.isInState(expectedStates)) {
throw new DoNotRetryRegionException(UNEXPECTED_STATE_REGION + regionNode);
}

if (isTableDisabled(regionNode.getTable())) {
throw new DoNotRetryIOException(regionNode.getTable() + " is disabled for " + regionNode);
}
Expand All @@ -786,6 +787,10 @@ private TransitRegionStateProcedure createAssignProcedure(RegionInfo regionInfo,
RegionStateNode regionNode = regionStates.getOrCreateRegionStateNode(regionInfo);
regionNode.lock();
try {
if (regionNode.isSplit()) {
throw new DoNotRetryRegionException(regionNode.getRegionInfo().getEncodedName()
+ " is a split parent and cannot be assigned");
}
if (override) {
if (!force) {
preTransitCheck(regionNode, STATES_EXPECTED_ON_ASSIGN);
Expand Down
2 changes: 2 additions & 0 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,8 @@ public void splitRegion(RegionInfo parent, RegionInfo splitA, RegionInfo splitB,
Put putParent = MetaTableAccessor.makePutFromRegionInfo(
RegionInfoBuilder.newBuilder(parent).setOffline(true).setSplit(true).build(), time);
MetaTableAccessor.addDaughtersToPut(putParent, splitA, splitB);
MetaTableAccessor.addRegionStateToPut(putParent, RegionInfo.DEFAULT_REPLICA_ID,

@tanyastickles tanyastickles Sep 24, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

this is the main thing i want feedback on. how do we feel about persisting state=SPLIT to the metafile in addition to the split=true flag?

@tanyastickles tanyastickles Sep 24, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

correct me if i'm wrong, but it looks pretty common to have drift between in-memory state and state that's written to the metafile.

SPLITTING, SPLITTING_NEW, MERGING, FAILED_OPEN, and some others are all in memory state that don't get written to the meta. those are more transient than SPLIT.

we already keep track of whether the region is split in split=true, so i don't have have a strong opinion on approach here.

@wchevreuil wchevreuil Sep 25, 2026

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.

So if we add this here, we wouldn't need the extra "isSplit" checks you added on AssignmentManager?

@tanyastickles tanyastickles Sep 25, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

we'd still need the isSplit check in AssignmentManager.createAssignProcedure (line 803). if the method is called with override=true, it would skip the preTransitCheck that validates that the region is CLOSED or OFFLINE.

@tanyastickles tanyastickles Sep 25, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

but elsewhere, we wouldn't need to add the additional isSplit checks

RegionState.State.SPLIT);

// Puts for daughters
Put putA = MetaTableAccessor.makePutFromRegionInfo(splitA, time);
Expand Down
28 changes: 28 additions & 0 deletions ...server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.Collections;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import org.apache.hadoop.hbase.DoNotRetryIOException;
import org.apache.hadoop.hbase.HBaseTestingUtil;
import org.apache.hadoop.hbase.MetaTableAccessor;
import org.apache.hadoop.hbase.TableName;
Expand Down Expand Up @@ -217,6 +219,32 @@ public void testUnassignAnUnassignedRegion() throws Exception {
assertEquals(unassignFailedCount, unassignProcMetrics.getFailedCounter().getCount());
}

@Test
public void testAssignThrowsWithASplitParent() throws Exception {
RegionInfo splitParent = RegionInfoBuilder.newBuilder(TableName.valueOf("test-split-split"))
.setSplit(true).setOffline(true).build();
RegionStateNode rsn = am.getRegionStates().getOrCreateRegionStateNode(splitParent);
rsn.setState(State.SPLIT);

assertThrows(DoNotRetryIOException.class, () -> am.assign(splitParent));
assertNull(am.createOneAssignProcedure(splitParent, true, false));
}

// Simulate the pre-fix failover scenario: regionInfo.isSplit()=true but state=OFFLINE because
// the old code path did not write SPLIT to info:state, causing loadMeta to fall back to OFFLINE.
// See HBASE-30353.
@Test
public void testAssignThrowsWithASplitParentInOfflineState() throws Exception {
RegionInfo splitParent =
RegionInfoBuilder.newBuilder(TableName.valueOf("test-split-offline")).setSplit(true)
.setOffline(true).build();
RegionStateNode rsn = am.getRegionStates().getOrCreateRegionStateNode(splitParent);
rsn.setState(State.OFFLINE);

assertThrows(DoNotRetryIOException.class, () -> am.assign(splitParent));
assertNull(am.createOneAssignProcedure(splitParent, true, false));
}

/**
* It is possible that when AM send assign meta request to a RS successfully, but RS can not send
* back any response, which cause master startup hangs forever
Expand Down
32 changes: 32 additions & 0 deletions ...-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionStateStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,38 @@ public void testEmptyMetaDaughterLocationDuringSplit() throws IOException {
}
}


@Test
public void testSplitRegionWritesSplitStateForParentInMeta() throws IOException {
long regionId = EnvironmentEdgeManager.currentTime();
ServerName serverName = ServerName.valueOf("foo", 60010, ThreadLocalRandom.current().nextLong());
TableName tableName = name.getTableName();
RegionInfo parent = RegionInfoBuilder.newBuilder(tableName)
.setStartKey(HConstants.EMPTY_START_ROW).setEndKey(HConstants.EMPTY_END_ROW).setSplit(false)
.setRegionId(regionId).setReplicaId(0).build();
RegionInfo splitA = RegionInfoBuilder.newBuilder(tableName)
.setStartKey(HConstants.EMPTY_START_ROW).setEndKey(Bytes.toBytes("a")).setSplit(false)
.setRegionId(regionId + 1).setReplicaId(0).build();
RegionInfo splitB = RegionInfoBuilder.newBuilder(tableName).setStartKey(Bytes.toBytes("a"))
.setEndKey(HConstants.EMPTY_END_ROW).setSplit(false).setRegionId(regionId + 1).setReplicaId(0)
.build();
MetaTableAccessor.addRegionsToMeta(UTIL.getConnection(), Lists.newArrayList(parent), 1);
final RegionStateStore regionStateStore =
UTIL.getHBaseCluster().getMaster().getAssignmentManager().getRegionStateStore();
regionStateStore.splitRegion(parent, splitA, splitB, serverName,
TableDescriptorBuilder.newBuilder(tableName).build());


try (Table meta = MetaTableAccessor.getMetaHTable(UTIL.getConnection())) {
Result result = meta.get(new Get(parent.getRegionName()));
Cell stateCell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY,
CatalogFamilyFormat.getRegionStateColumn(RegionInfo.DEFAULT_REPLICA_ID));
assertNotNull(stateCell);
assertEquals(RegionState.State.SPLIT.name(), Bytes.toString(stateCell.getValueArray(),
stateCell.getValueOffset(), stateCell.getValueLength()));
}
}

@Test
public void testMetaLocationForRegionReplicasIsAddedAtRegionMerge() throws IOException {
long regionId = EnvironmentEdgeManager.currentTime();
Expand Down
Loading