Skip to content

Commit e3da929

Browse files
hingu-8103hingu-8103
authored
HBASE-30160 Prevent region creation if the encoded region names are the same (#8395)
Signed-off-by: Balazs Meszaros <meszibalu@apache.org>
1 parent 6efcf6c commit e3da929

5 files changed

Lines changed: 173 additions & 2 deletions

File tree

‎hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,14 @@ public List<RegionInfo> getRegionsOfTableForEnabling(TableName table) {
330330
regionNode -> !regionNode.isInState(State.SPLIT) && !regionNode.getRegionInfo().isSplit());
331331
}
332332

333+
/**
334+
* Returns all regions of the table, irrespective of assignment or split/offline state.
335+
*/
336+
public List<RegionInfo> getAllRegionsOfTable(TableName table) {
337+
return getTableRegionStateNodes(table).stream().map(RegionStateNode::getRegionInfo)
338+
.collect(Collectors.toList());
339+
}
340+
333341
/**
334342
* Get the regions for deleting a table.
335343
* <p/>
@@ -338,8 +346,7 @@ public List<RegionInfo> getRegionsOfTableForEnabling(TableName table) {
338346
* references to the regions, we will lose the data of the regions.
339347
*/
340348
public List<RegionInfo> getRegionsOfTableForDeleting(TableName table) {
341-
return getTableRegionStateNodes(table).stream().map(RegionStateNode::getRegionInfo)
342-
.collect(Collectors.toList());
349+
return getAllRegionsOfTable(table);
343350
}
344351

345352
/** Returns Return the regions of the table and filter them. */

‎hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
import org.apache.hadoop.hbase.util.Bytes;
7272
import org.apache.hadoop.hbase.util.CommonFSUtils;
7373
import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
74+
import org.apache.hadoop.hbase.util.ModifyRegionUtils;
7475
import org.apache.hadoop.hbase.util.Pair;
7576
import org.apache.hadoop.hbase.util.Threads;
7677
import org.apache.hadoop.hbase.wal.WALSplitUtil;
@@ -141,6 +142,8 @@ public SplitTableRegionProcedure(final MasterProcedureEnv env, final RegionInfo
141142
.setEndKey(bestSplitRow).setSplit(false).setRegionId(rid).build();
142143
this.daughterTwoRI = RegionInfoBuilder.newBuilder(table).setStartKey(bestSplitRow)
143144
.setEndKey(regionToSplit.getEndKey()).setSplit(false).setRegionId(rid).build();
145+
ModifyRegionUtils.checkForEncodedNameCollisions(Arrays.asList(daughterOneRI, daughterTwoRI),
146+
env.getAssignmentManager().getRegionStates());
144147

145148
if (tableDescriptor.getRegionSplitPolicyClassName() != null) {
146149
// Since we don't have region reference here, creating the split policy instance without it.

‎hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,14 @@ private boolean prepareCreate(final MasterProcedureEnv env) throws IOException {
296296
return false;
297297
}
298298

299+
try {
300+
ModifyRegionUtils.checkForEncodedNameCollisions(newRegions,
301+
env.getAssignmentManager().getRegionStates());
302+
} catch (DoNotRetryIOException e) {
303+
setFailure("master-create-table", e);
304+
return false;
305+
}
306+
299307
if (!tableName.isSystemTable()) {
300308
// do not check rs group for system tables as we may block the bootstrap.
301309
Supplier<String> forWhom = () -> "table " + tableName;

‎hbase-server/src/main/java/org/apache/hadoop/hbase/util/ModifyRegionUtils.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@
2121
import java.io.InterruptedIOException;
2222
import java.util.ArrayList;
2323
import java.util.Collection;
24+
import java.util.HashSet;
2425
import java.util.List;
26+
import java.util.Objects;
27+
import java.util.Set;
2528
import java.util.concurrent.Callable;
2629
import java.util.concurrent.CompletionService;
2730
import java.util.concurrent.ExecutionException;
@@ -30,10 +33,12 @@
3033
import java.util.concurrent.TimeUnit;
3134
import org.apache.hadoop.conf.Configuration;
3235
import org.apache.hadoop.fs.Path;
36+
import org.apache.hadoop.hbase.DoNotRetryIOException;
3337
import org.apache.hadoop.hbase.HConstants;
3438
import org.apache.hadoop.hbase.client.RegionInfo;
3539
import org.apache.hadoop.hbase.client.RegionInfoBuilder;
3640
import org.apache.hadoop.hbase.client.TableDescriptor;
41+
import org.apache.hadoop.hbase.master.assignment.RegionStates;
3742
import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv;
3843
import org.apache.hadoop.hbase.regionserver.HRegion;
3944
import org.apache.yetus.audience.InterfaceAudience;
@@ -82,6 +87,29 @@ public static RegionInfo[] createRegionInfos(TableDescriptor tableDescriptor,
8287
return hRegionInfos;
8388
}
8489

90+
/**
91+
* Checks candidate regions for encoded\-name collisions. Ensures there are no duplicates in the
92+
* input and no conflicts with existing region states.
93+
*/
94+
public static void checkForEncodedNameCollisions(final Collection<RegionInfo> candidates,
95+
final RegionStates regionStates) throws IOException {
96+
if (candidates == null || candidates.isEmpty()) {
97+
return;
98+
}
99+
Objects.requireNonNull(regionStates, "regionStates is null");
100+
Set<String> candidateNames = new HashSet<>();
101+
for (RegionInfo ri : candidates) {
102+
String encoded = ri.getEncodedName();
103+
if (
104+
!candidateNames.add(encoded)
105+
|| regionStates.getRegionStateNodeFromEncodedRegionName(encoded) != null
106+
) {
107+
throw new DoNotRetryIOException("Encoded region name collision detected: '" + encoded
108+
+ "' for table " + ri.getTable() + ". Refusing to proceed.");
109+
}
110+
}
111+
}
112+
85113
/**
86114
* Create new set of regions on the specified file-system. NOTE: that you should add the regions
87115
* to hbase:meta after this operation.

‎hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestEncodedNameCollisionDetection.java‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.hadoop.hbase.master.procedure;
19+
20+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
21+
import static org.junit.jupiter.api.Assertions.assertThrows;
22+
import static org.junit.jupiter.api.Assertions.assertTrue;
23+
import static org.mockito.Mockito.mock;
24+
import static org.mockito.Mockito.when;
25+
26+
import java.util.Arrays;
27+
import java.util.Collections;
28+
import org.apache.hadoop.hbase.DoNotRetryIOException;
29+
import org.apache.hadoop.hbase.TableName;
30+
import org.apache.hadoop.hbase.client.RegionInfo;
31+
import org.apache.hadoop.hbase.client.RegionInfoBuilder;
32+
import org.apache.hadoop.hbase.master.assignment.RegionStateNode;
33+
import org.apache.hadoop.hbase.master.assignment.RegionStates;
34+
import org.apache.hadoop.hbase.testclassification.SmallTests;
35+
import org.apache.hadoop.hbase.util.ModifyRegionUtils;
36+
import org.junit.jupiter.api.Tag;
37+
import org.junit.jupiter.api.Test;
38+
39+
/**
40+
* Tests for encoded region-name collision detection (HBASE-30160). If two regions end up with the
41+
* same encoded name, we should fail fast instead of allowing subtle metadata corruption later.
42+
*/
43+
44+
@Tag(SmallTests.TAG)
45+
public class TestEncodedNameCollisionDetection {
46+
47+
/**
48+
* Happy-path check: distinct candidate regions should pass without throwing.
49+
*/
50+
@Test
51+
public void testAcceptsDistinctCandidates() {
52+
TableName tableName = TableName.valueOf("test_table");
53+
long regionId = System.currentTimeMillis();
54+
55+
RegionInfo ri1 = RegionInfoBuilder.newBuilder(tableName).setStartKey(new byte[] { 0, 0 })
56+
.setEndKey(new byte[] { 1, 0 }).setSplit(false).setRegionId(regionId).build();
57+
58+
RegionInfo ri2 = RegionInfoBuilder.newBuilder(tableName).setStartKey(new byte[] { 1, 0 })
59+
.setEndKey(new byte[] { 2, 0 }).setSplit(false).setRegionId(regionId + 1).build();
60+
61+
RegionStates regionStates = mock(RegionStates.class);
62+
63+
assertDoesNotThrow(
64+
() -> ModifyRegionUtils.checkForEncodedNameCollisions(Arrays.asList(ri1, ri2), regionStates));
65+
}
66+
67+
/**
68+
* Verifies that duplicate encoded region names within candidate regions are rejected.
69+
*/
70+
@Test
71+
public void testDetectsDuplicatesInCandidates() {
72+
TableName tableName = TableName.valueOf("test_table");
73+
RegionInfo ri1 = mockRegionInfo(tableName, "same-encoded-name");
74+
RegionInfo ri2 = mockRegionInfo(tableName, "same-encoded-name");
75+
76+
RegionStates regionStates = mock(RegionStates.class);
77+
78+
DoNotRetryIOException exception = assertThrows(DoNotRetryIOException.class,
79+
() -> ModifyRegionUtils.checkForEncodedNameCollisions(Arrays.asList(ri1, ri2), regionStates));
80+
assertTrue(exception.getMessage().contains("Encoded region name collision detected"));
81+
}
82+
83+
/**
84+
* A candidate region should be rejected if its encoded name already exists.
85+
*/
86+
@Test
87+
public void testDetectsCollisionWithExistingRegions() {
88+
TableName tableName = TableName.valueOf("test_table");
89+
RegionInfo candidateRegion = mockRegionInfo(tableName, "same-encoded-name");
90+
RegionStates regionStates = mock(RegionStates.class);
91+
when(regionStates.getRegionStateNodeFromEncodedRegionName("same-encoded-name"))
92+
.thenReturn(mock(RegionStateNode.class));
93+
94+
DoNotRetryIOException exception =
95+
assertThrows(DoNotRetryIOException.class, () -> ModifyRegionUtils
96+
.checkForEncodedNameCollisions(Arrays.asList(candidateRegion), regionStates));
97+
assertTrue(exception.getMessage().contains("Encoded region name collision detected"));
98+
}
99+
100+
/**
101+
* Test that checkForEncodedNameCollisions handles empty/null inputs and rejects null RegionStates
102+
* when candidates are present.
103+
*/
104+
@Test
105+
public void testInputValidationAndNullRegionStatesBehavior() {
106+
assertDoesNotThrow(() -> ModifyRegionUtils.checkForEncodedNameCollisions(null, null));
107+
assertDoesNotThrow(
108+
() -> ModifyRegionUtils.checkForEncodedNameCollisions(Collections.emptyList(), null));
109+
assertDoesNotThrow(
110+
() -> ModifyRegionUtils.checkForEncodedNameCollisions(null, mock(RegionStates.class)));
111+
assertDoesNotThrow(() -> ModifyRegionUtils
112+
.checkForEncodedNameCollisions(Collections.emptyList(), mock(RegionStates.class)));
113+
114+
RegionInfo candidateRegion = mockRegionInfo(TableName.valueOf("test_table"), "candidate");
115+
assertThrows(NullPointerException.class,
116+
() -> ModifyRegionUtils.checkForEncodedNameCollisions(Arrays.asList(candidateRegion), null));
117+
}
118+
119+
private RegionInfo mockRegionInfo(TableName tableName, String encodedName) {
120+
RegionInfo regionInfo = mock(RegionInfo.class);
121+
when(regionInfo.getEncodedName()).thenReturn(encodedName);
122+
when(regionInfo.getTable()).thenReturn(tableName);
123+
return regionInfo;
124+
}
125+
}

0 commit comments

Comments
 (0)