You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2019/08/29 19:53:48 UTC
[hbase] branch branch-2.2 updated: HBASE-22941 merge operation
returns parent regions in random order (#556)
This is an automated email from the ASF dual-hosted git repository.
stack pushed a commit to branch branch-2.2
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2.2 by this push:
new 047aad1 HBASE-22941 merge operation returns parent regions in random order (#556)
047aad1 is described below
commit 047aad1e84775b3e75636b2780af775ffc9e4cba
Author: Istvan Toth <st...@users.noreply.github.com>
AuthorDate: Thu Aug 29 21:27:33 2019 +0200
HBASE-22941 merge operation returns parent regions in random order (#556)
* HBASE-22941 merge operation returns parent regions in random order
store and return the merge parent regions in ascending order
remove left over check for exactly two merged regions
add unit test
* use SortedMap type to emphasise that the Map is sorted.
* use regionCount consistently and checkstyle fixes
* Delete tests that expect multiregion merges to fail.
Signed-off-by: stack <st...@apache.org>
---
.../hadoop/hbase/master/MasterRpcServices.java | 4 --
.../hbase/master/assignment/RegionStateStore.java | 6 +--
.../org/apache/hadoop/hbase/TestSplitMerge.java | 49 ++++++++++++++++++++++
.../org/apache/hadoop/hbase/client/TestAdmin1.java | 10 -----
.../hbase/client/TestAsyncRegionAdminApi2.java | 11 -----
5 files changed, 52 insertions(+), 28 deletions(-)
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
index 5823fa7..2ff036c 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
@@ -805,10 +805,6 @@ public class MasterRpcServices extends RSRpcServices
RegionStates regionStates = master.getAssignmentManager().getRegionStates();
- if (request.getRegionCount() != 2) {
- throw new ServiceException(new DoNotRetryIOException(
- "Only support merging 2 regions but " + request.getRegionCount() + " region passed"));
- }
RegionInfo[] regionsToMerge = new RegionInfo[request.getRegionCount()];
for (int i = 0; i < request.getRegionCount(); i++) {
final byte[] encodedNameOfRegion = request.getRegion(i).getValue().toByteArray();
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java
index fc88a75..dcc38c8 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java
@@ -19,9 +19,9 @@ package org.apache.hadoop.hbase.master.assignment;
import java.io.IOException;
import java.util.Collections;
-import java.util.HashMap;
import java.util.List;
-import java.util.Map;
+import java.util.SortedMap;
+import java.util.TreeMap;
import org.apache.hadoop.hbase.Cell;
import org.apache.hadoop.hbase.CellBuilderFactory;
@@ -246,7 +246,7 @@ public class RegionStateStore {
throws IOException {
TableDescriptor htd = getTableDescriptor(child.getTable());
boolean globalScope = htd.hasGlobalReplicationScope();
- Map<RegionInfo, Long> parentSeqNums = new HashMap<>(parents.length);
+ SortedMap<RegionInfo, Long> parentSeqNums = new TreeMap<>();
for (RegionInfo ri: parents) {
parentSeqNums.put(ri, globalScope? getOpenSeqNumForParentRegion(ri): -1);
}
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestSplitMerge.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestSplitMerge.java
index 641e52e..0aa3297 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestSplitMerge.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestSplitMerge.java
@@ -19,7 +19,9 @@ package org.apache.hadoop.hbase;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import java.util.List;
import java.util.concurrent.TimeUnit;
import org.apache.hadoop.hbase.Waiter.ExplainingPredicate;
import org.apache.hadoop.hbase.client.AsyncConnection;
@@ -104,4 +106,51 @@ public class TestSplitMerge {
.getRegionLocation(Bytes.toBytes(1), true).get().getServerName());
}
}
+
+ @Test
+ public void testMergeRegionOrder() throws Exception {
+
+ int regionCount= 20;
+
+ TableName tableName = TableName.valueOf("MergeRegionOrder");
+ byte[] family = Bytes.toBytes("CF");
+ TableDescriptor td = TableDescriptorBuilder.newBuilder(tableName)
+ .setColumnFamily(ColumnFamilyDescriptorBuilder.of(family)).build();
+
+ byte[][] splitKeys = new byte[regionCount-1][];
+
+ for (int c = 0; c < regionCount-1; c++) {
+ splitKeys[c] = Bytes.toBytes(c+1 * 1000);
+ }
+
+ UTIL.getAdmin().createTable(td, splitKeys);
+ UTIL.waitTableAvailable(tableName);
+
+ List<RegionInfo> regions = UTIL.getAdmin().getRegions(tableName);
+
+ byte[][] regionNames = new byte[regionCount][];
+ for (int c = 0; c < regionCount; c++) {
+ regionNames[c] = regions.get(c).getRegionName();
+ }
+
+ UTIL.getAdmin().mergeRegionsAsync(regionNames, false).get(60, TimeUnit.SECONDS);
+
+ List<RegionInfo> mergedRegions =
+ MetaTableAccessor.getTableRegions(UTIL.getConnection(), tableName);
+
+ assertEquals(1, mergedRegions.size());
+
+ RegionInfo mergedRegion = mergedRegions.get(0);
+
+ List<RegionInfo> mergeParentRegions = MetaTableAccessor.getMergeRegions(UTIL.getConnection(),
+ mergedRegion.getEncodedNameAsBytes());
+
+ assertEquals(mergeParentRegions.size(), regionCount);
+
+ for (int c = 0; c < regionCount-1; c++) {
+ assertTrue(Bytes.compareTo(
+ mergeParentRegions.get(c).getStartKey(),
+ mergeParentRegions.get(c+1).getStartKey()) < 0);
+ }
+ }
}
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
index bbff17a..c552fe6 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
@@ -32,7 +32,6 @@ import java.util.Map;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
-import org.apache.hadoop.hbase.DoNotRetryIOException;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseTestingUtility;
import org.apache.hadoop.hbase.HColumnDescriptor;
@@ -1467,15 +1466,6 @@ public class TestAdmin1 {
} catch (IllegalArgumentException e) {
// expected
}
- // 3
- try {
- admin.mergeRegionsAsync(
- tableRegions.stream().map(RegionInfo::getEncodedNameAsBytes).toArray(byte[][]::new),
- false).get();
- fail();
- } catch (DoNotRetryIOException e) {
- // expected
- }
} finally {
admin.disableTable(tableName);
admin.deleteTable(tableName);
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionAdminApi2.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionAdminApi2.java
index 7d6d2e0..8616ed5 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionAdminApi2.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionAdminApi2.java
@@ -32,7 +32,6 @@ import java.util.Optional;
import java.util.concurrent.ExecutionException;
import java.util.stream.Collectors;
import org.apache.hadoop.hbase.AsyncMetaTableAccessor;
-import org.apache.hadoop.hbase.DoNotRetryIOException;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.HRegionLocation;
@@ -205,16 +204,6 @@ public class TestAsyncRegionAdminApi2 extends TestAsyncAdminBase {
// expected
assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
}
- // 3
- try {
- admin.mergeRegions(
- regions.stream().map(RegionInfo::getEncodedNameAsBytes).collect(Collectors.toList()), false)
- .get();
- fail();
- } catch (ExecutionException e) {
- // expected
- assertThat(e.getCause(), instanceOf(DoNotRetryIOException.class));
- }
}
@Test