From 2e80896842b95f08f8a23b511bdffcd0642c280f Mon Sep 17 00:00:00 2001 From: guluo Date: Sun, 17 Mar 2024 10:25:43 +0800 Subject: [PATCH 1/6] Fix issue of returning too slow --- .../hbase/client/AsyncRegionLocator.java | 78 ++++++--- ...estScanTableWithReplicationFromClient.java | 153 ++++++++++++++++++ 2 files changed, 207 insertions(+), 24 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScanTableWithReplicationFromClient.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java index da58dd8e1e53..eff475203322 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java @@ -35,6 +35,7 @@ import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; +import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HRegionLocation; import org.apache.hadoop.hbase.RegionLocations; import org.apache.hadoop.hbase.ServerName; @@ -151,36 +152,65 @@ CompletableFuture getRegionLocations(TableName tableName, byte[ }, AsyncRegionLocator::getRegionNames, supplier); } + public void internalAddListener(CompletableFuture future, + CompletableFuture locsFuture, TableName tableName, byte[] row, int replicaId, + RegionLocateType type) { + addListener(locsFuture, (locs, error) -> { + if (error != null) { + future.completeExceptionally(error); + return; + } + HRegionLocation loc = locs.getRegionLocation(replicaId); + if (loc == null) { + future.completeExceptionally( + new RegionOfflineException("No location for " + tableName + ", row='" + + Bytes.toStringBinary(row) + "', locateType=" + type + ", replicaId=" + replicaId)); + } else if (loc.getServerName() == null) { + future + .completeExceptionally(new RegionOfflineException("No server address listed for region '" + + loc.getRegion().getRegionNameAsString() + ", row='" + Bytes.toStringBinary(row) + + "', locateType=" + type + ", replicaId=" + replicaId)); + } else { + future.complete(loc); + } + }); + } + CompletableFuture getRegionLocation(TableName tableName, byte[] row, int replicaId, RegionLocateType type, boolean reload, long timeoutNs) { final Supplier supplier = new TableSpanBuilder(conn) .setName("AsyncRegionLocator.getRegionLocation").setTableName(tableName); return tracedLocationFuture(() -> { - // meta region can not be split right now so we always call the same method. - // Change it later if the meta table can have more than one regions. CompletableFuture future = new CompletableFuture<>(); - CompletableFuture locsFuture = isMeta(tableName) - ? metaRegionLocator.getRegionLocations(replicaId, reload) - : nonMetaRegionLocator.getRegionLocations(tableName, row, replicaId, type, reload); - addListener(locsFuture, (locs, error) -> { - if (error != null) { - future.completeExceptionally(error); - return; - } - HRegionLocation loc = locs.getRegionLocation(replicaId); - if (loc == null) { - future.completeExceptionally( - new RegionOfflineException("No location for " + tableName + ", row='" - + Bytes.toStringBinary(row) + "', locateType=" + type + ", replicaId=" + replicaId)); - } else if (loc.getServerName() == null) { - future.completeExceptionally( - new RegionOfflineException("No server address listed for region '" - + loc.getRegion().getRegionNameAsString() + ", row='" + Bytes.toStringBinary(row) - + "', locateType=" + type + ", replicaId=" + replicaId)); - } else { - future.complete(loc); - } - }); + if (replicaId == RegionReplicaUtil.DEFAULT_REPLICA_ID) { + // meta region can not be split right now so we always call the same method. + // Change it later if the meta table can have more than one regions. + CompletableFuture locsFuture = isMeta(tableName) + ? metaRegionLocator.getRegionLocations(replicaId, reload) + : nonMetaRegionLocator.getRegionLocations(tableName, row, replicaId, type, reload); + internalAddListener(future, locsFuture, tableName, row, replicaId, type); + } else { + addListener(conn.getAdmin().getDescriptor(tableName), (tdesc, error) -> { + if (error != null) { + future.completeExceptionally(error); + return; + } + int regionReplicationCount = tdesc.getRegionReplication(); + if (replicaId >= regionReplicationCount) { + future + .completeExceptionally(new DoNotRetryIOException("The specified region replica id '" + + replicaId + "' does not exist, the region replication of this table " + + tableName.getNameAsString() + " is " + regionReplicationCount)); + return; + } + // meta region can not be split right now so we always call the same method. + // Change it later if the meta table can have more than one regions. + CompletableFuture locsFuture = isMeta(tableName) + ? metaRegionLocator.getRegionLocations(replicaId, reload) + : nonMetaRegionLocator.getRegionLocations(tableName, row, replicaId, type, reload); + internalAddListener(future, locsFuture, tableName, row, replicaId, type); + }); + } return withTimeout(future, timeoutNs, () -> "Timeout(" + TimeUnit.NANOSECONDS.toMillis(timeoutNs) + "ms) waiting for region location for " + tableName + ", row='" diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScanTableWithReplicationFromClient.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScanTableWithReplicationFromClient.java new file mode 100644 index 000000000000..6c9048e817ce --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScanTableWithReplicationFromClient.java @@ -0,0 +1,153 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.client; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.util.List; +import org.apache.hadoop.hbase.DoNotRetryIOException; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseTestingUtil; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.filter.PrefixFilter; +import org.apache.hadoop.hbase.regionserver.HRegion; +import org.apache.hadoop.hbase.testclassification.ClientTests; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.io.IOUtils; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.rules.ExpectedException; +import org.junit.rules.TestName; + +@Category({ MediumTests.class, ClientTests.class }) +public class TestScanTableWithReplicationFromClient { + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestScanTableWithReplicationFromClient.class); + + @Rule + public TestName name = new TestName(); + + private ExpectedException exception = ExpectedException.none(); + + private final static HBaseTestingUtil UTIL = new HBaseTestingUtil(); + private static final String ROW = "r1"; + private static final byte[] FAMILY = Bytes.toBytes("info"); + private static final int REGION_REPLICATION_COUNT = 2; + // region replica id starts from 0 + private static final int NON_EXISTING_REGION_REPLICA_ID = REGION_REPLICATION_COUNT; + private static Connection connection; + private static Admin admin; + private TableName tableName; + + @BeforeClass + public static void setUpBeforeClass() throws Exception { + UTIL.startMiniCluster(1); + connection = UTIL.getConnection(); + admin = UTIL.getAdmin(); + } + + @AfterClass + public static void tearDownAfterClass() throws Exception { + IOUtils.cleanup(null, admin); + IOUtils.cleanup(null, connection); + UTIL.shutdownMiniCluster(); + } + + @Before + public void setUp() throws Exception { + tableName = TableName.valueOf(name.getMethodName()); + ColumnFamilyDescriptor columnFamilyDescriptor = + ColumnFamilyDescriptorBuilder.newBuilder(FAMILY).build(); + TableDescriptor tableDescriptor = + TableDescriptorBuilder.newBuilder(tableName).setColumnFamily(columnFamilyDescriptor) + .setRegionReplication(REGION_REPLICATION_COUNT).build(); + admin.createTable(tableDescriptor); + UTIL.waitTableAvailable(tableName); + assertTrue(admin.tableExists(tableName)); + assertEquals(REGION_REPLICATION_COUNT, tableDescriptor.getRegionReplication()); + + List regions = UTIL.getHBaseCluster().getRegions(tableName); + assertEquals(REGION_REPLICATION_COUNT, regions.size()); + + Table table = connection.getTable(tableName); + Put put = new Put(Bytes.toBytes(ROW)).addColumn(FAMILY, Bytes.toBytes("q"), + Bytes.toBytes("test_value")); + table.put(put); + admin.flush(tableName); + + Scan scan = new Scan(); + ResultScanner rs = table.getScanner(scan); + rs.forEach(r -> assertEquals(ROW, r.getRow().toString())); + } + + @After + public void tearDown() throws Exception { + UTIL.deleteTable(tableName); + } + + @Test + public void testScanMetaWithRegionReplicaId() throws IOException { + Table metaTable = connection.getTable(TableName.META_TABLE_NAME); + Scan scan = new Scan(); + scan.setFilter(new PrefixFilter(tableName.getName())); + scan.setReplicaId(RegionReplicaUtil.DEFAULT_REPLICA_ID); + scan.setConsistency(Consistency.TIMELINE); + ResultScanner rs = metaTable.getScanner(scan); + rs.forEach(r -> assertTrue(r.getRow().toString().contains(tableName.getNameAsString()))); + } + + @Test + public void testScanMetaWithNonExistingRegionReplicaId() throws IOException { + Table metaTable = connection.getTable(TableName.META_TABLE_NAME); + Scan scan = new Scan(); + scan.setReplicaId(NON_EXISTING_REGION_REPLICA_ID); + scan.setConsistency(Consistency.TIMELINE); + exception.expect(DoNotRetryIOException.class); + metaTable.getScanner(scan); + } + + @Test + public void testScanTableWithRegionReplicaId() throws IOException { + Table table = connection.getTable(tableName); + Scan scan = new Scan(); + scan.setReplicaId(RegionReplicaUtil.DEFAULT_REPLICA_ID); + scan.setConsistency(Consistency.TIMELINE); + ResultScanner rs = table.getScanner(scan); + rs.forEach(r -> assertEquals(ROW, r.getRow().toString())); + } + + @Test + public void testScanTableWithNonExistingRegionReplicaId() throws IOException { + Table table = connection.getTable(tableName); + Scan scan = new Scan(); + scan.setReplicaId(NON_EXISTING_REGION_REPLICA_ID); + scan.setConsistency(Consistency.TIMELINE); + exception.expect(DoNotRetryIOException.class); + table.getScanner(scan); + } +} From 301793045ffa086f28db5c15ad1c8d3c83765487 Mon Sep 17 00:00:00 2001 From: guluo Date: Sun, 17 Mar 2024 10:50:55 +0800 Subject: [PATCH 2/6] update message --- .../org/apache/hadoop/hbase/client/AsyncRegionLocator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java index eff475203322..f8f9f6323e1c 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java @@ -199,8 +199,8 @@ CompletableFuture getRegionLocation(TableName tableName, byte[] if (replicaId >= regionReplicationCount) { future .completeExceptionally(new DoNotRetryIOException("The specified region replica id '" - + replicaId + "' does not exist, the region replication of this table " - + tableName.getNameAsString() + " is " + regionReplicationCount)); + + replicaId + "' does not exist, the REGION_REPLICATION of this table " + + tableName.getNameAsString() + " is " + regionReplicationCount + " .")); return; } // meta region can not be split right now so we always call the same method. From cf6443608770be4803bed169517626d6c0dd720e Mon Sep 17 00:00:00 2001 From: guluo Date: Sun, 17 Mar 2024 23:14:12 +0800 Subject: [PATCH 3/6] update unit test --- .../hbase/client/AsyncRegionLocator.java | 4 +- ...TableRegionLocatorWithRegionReplicaId.java | 154 ++++++++++++++++++ ...estScanTableWithReplicationFromClient.java | 33 ++-- 3 files changed, 180 insertions(+), 11 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableRegionLocatorWithRegionReplicaId.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java index f8f9f6323e1c..834fb1d030c9 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java @@ -200,7 +200,9 @@ CompletableFuture getRegionLocation(TableName tableName, byte[] future .completeExceptionally(new DoNotRetryIOException("The specified region replica id '" + replicaId + "' does not exist, the REGION_REPLICATION of this table " - + tableName.getNameAsString() + " is " + regionReplicationCount + " .")); + + tableName.getNameAsString() + " is " + regionReplicationCount + " , this means " + + "that the maximum region replica id you can specify is " + + (regionReplicationCount - 1) + ".")); return; } // meta region can not be split right now so we always call the same method. diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableRegionLocatorWithRegionReplicaId.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableRegionLocatorWithRegionReplicaId.java new file mode 100644 index 000000000000..ccdf253f12c5 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableRegionLocatorWithRegionReplicaId.java @@ -0,0 +1,154 @@ +package org.apache.hadoop.hbase.client; + +import org.apache.hadoop.hbase.DoNotRetryIOException; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseTestingUtil; +import org.apache.hadoop.hbase.HRegionLocation; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.regionserver.HRegion; +import org.apache.hadoop.hbase.testclassification.ClientTests; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.io.IOUtils; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.rules.ExpectedException; +import org.junit.rules.TestName; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +@Category({ MediumTests.class, ClientTests.class }) +public class TestAsyncTableRegionLocatorWithRegionReplicaId { + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestAsyncTableRegionLocatorWithRegionReplicaId.class); + + @Rule + public TestName name = new TestName(); + + private ExpectedException exception = ExpectedException.none(); + + private final static HBaseTestingUtil UTIL = new HBaseTestingUtil(); + private static final String ROW = "r1"; + private static final byte[] FAMILY = Bytes.toBytes("info"); + private static final int REGION_REPLICATION_COUNT = 2; + // region replica id starts from 0 + private static final int NON_EXISTING_REGION_REPLICA_ID = REGION_REPLICATION_COUNT; + private static Connection connection; + private static AsyncConnection asyncConn; + private static Admin admin; + private TableName tableName; + + @BeforeClass + public static void setUpBeforeClass() throws Exception { + UTIL.startMiniCluster(1); + connection = UTIL.getConnection(); + asyncConn = ConnectionFactory.createAsyncConnection(UTIL.getConfiguration()).get(); + admin = UTIL.getAdmin(); + } + + @AfterClass + public static void tearDownAfterClass() throws Exception { + IOUtils.cleanup(null, admin); + IOUtils.cleanup(null, connection); + UTIL.shutdownMiniCluster(); + } + + @Before + public void setUp() throws Exception { + tableName = TableName.valueOf(name.getMethodName()); + ColumnFamilyDescriptor columnFamilyDescriptor = + ColumnFamilyDescriptorBuilder.newBuilder(FAMILY).build(); + TableDescriptor tableDescriptor = + TableDescriptorBuilder.newBuilder(tableName).setColumnFamily(columnFamilyDescriptor) + .setRegionReplication(REGION_REPLICATION_COUNT).build(); + admin.createTable(tableDescriptor); + UTIL.waitTableAvailable(tableName); + assertTrue(admin.tableExists(tableName)); + assertEquals(REGION_REPLICATION_COUNT, tableDescriptor.getRegionReplication()); + + List regions = UTIL.getHBaseCluster().getRegions(tableName); + assertEquals(REGION_REPLICATION_COUNT, regions.size()); + + Table table = connection.getTable(tableName); + Put put = new Put(Bytes.toBytes(ROW)).addColumn(FAMILY, Bytes.toBytes("q"), + Bytes.toBytes("test_value")); + table.put(put); + admin.flush(tableName); + + Scan scan = new Scan(); + ResultScanner rs = table.getScanner(scan); + rs.forEach(r -> assertEquals(ROW, Bytes.toString(r.getRow()))); + } + + @After + public void tearDown() throws Exception { + UTIL.deleteTable(tableName); + } + + @Test + public void testMetaTableRegionLocatorWithRegionReplicaId() + throws ExecutionException, InterruptedException { + AsyncTableRegionLocator locator = asyncConn.getRegionLocator(TableName.META_TABLE_NAME); + CompletableFuture + future = locator.getRegionLocation(tableName.getName(), RegionReplicaUtil.DEFAULT_REPLICA_ID, true); + HRegionLocation hrl = future.get(); + assertNotNull(hrl); + } + + @Test + public void testMetaTableRegionLocatorWithNonExistingRegionReplicaId() throws InterruptedException { + AsyncTableRegionLocator locator = asyncConn.getRegionLocator(TableName.META_TABLE_NAME); + CompletableFuture + future = locator.getRegionLocation(tableName.getName(), NON_EXISTING_REGION_REPLICA_ID, true); + try { + future.get(); + } catch (ExecutionException e) { + assertTrue(e.getCause() instanceof DoNotRetryIOException); + String message = "The specified region replica id '" + + NON_EXISTING_REGION_REPLICA_ID + "' does not exist, the REGION_REPLICATION of this " + + "table " + TableName.META_TABLE_NAME.getNameAsString() + " is " + + TableDescriptorBuilder.DEFAULT_REGION_REPLICATION + ", " + + "this means that the maximum region replica id you can specify is " + + (TableDescriptorBuilder.DEFAULT_REGION_REPLICATION - 1) + "."; + assertEquals(message, e.getCause().getMessage()); + } + } + + @Test + public void testTableRegionLocatorWithRegionReplicaId() + throws ExecutionException, InterruptedException { + AsyncTableRegionLocator locator = asyncConn.getRegionLocator(tableName); + CompletableFuture + future = locator.getRegionLocation(Bytes.toBytes(ROW), RegionReplicaUtil.DEFAULT_REPLICA_ID, true); + HRegionLocation hrl = future.get(); + assertNotNull(hrl); + } + + @Test + public void testTableRegionLocatorWithNonExistingRegionReplicaId() throws InterruptedException { + AsyncTableRegionLocator locator = asyncConn.getRegionLocator(tableName); + CompletableFuture future = locator.getRegionLocation(Bytes.toBytes(ROW), NON_EXISTING_REGION_REPLICA_ID, true); + try { + future.get(); + } catch (ExecutionException e) { + assertTrue(e.getCause() instanceof DoNotRetryIOException); + String message = "The specified region replica id '" + + NON_EXISTING_REGION_REPLICA_ID + "' does not exist, the REGION_REPLICATION of this " + + "table " + tableName.getNameAsString() + " is " + REGION_REPLICATION_COUNT + ", " + + "this means that the maximum region replica id you can specify is " + + (REGION_REPLICATION_COUNT - 1) + "."; + assertEquals(message, e.getCause().getMessage()); + } + } +} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScanTableWithReplicationFromClient.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScanTableWithReplicationFromClient.java index 6c9048e817ce..188bee65f777 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScanTableWithReplicationFromClient.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScanTableWithReplicationFromClient.java @@ -22,9 +22,12 @@ import java.io.IOException; import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtil; +import org.apache.hadoop.hbase.HRegionLocation; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.filter.PrefixFilter; import org.apache.hadoop.hbase.regionserver.HRegion; @@ -61,6 +64,7 @@ public class TestScanTableWithReplicationFromClient { // region replica id starts from 0 private static final int NON_EXISTING_REGION_REPLICA_ID = REGION_REPLICATION_COUNT; private static Connection connection; + private static AsyncConnection asyncConn; private static Admin admin; private TableName tableName; @@ -68,6 +72,7 @@ public class TestScanTableWithReplicationFromClient { public static void setUpBeforeClass() throws Exception { UTIL.startMiniCluster(1); connection = UTIL.getConnection(); + asyncConn = ConnectionFactory.createAsyncConnection(UTIL.getConfiguration()).get(); admin = UTIL.getAdmin(); } @@ -102,7 +107,7 @@ public void setUp() throws Exception { Scan scan = new Scan(); ResultScanner rs = table.getScanner(scan); - rs.forEach(r -> assertEquals(ROW, r.getRow().toString())); + rs.forEach(r -> assertEquals(ROW, Bytes.toString(r.getRow()))); } @After @@ -118,7 +123,7 @@ public void testScanMetaWithRegionReplicaId() throws IOException { scan.setReplicaId(RegionReplicaUtil.DEFAULT_REPLICA_ID); scan.setConsistency(Consistency.TIMELINE); ResultScanner rs = metaTable.getScanner(scan); - rs.forEach(r -> assertTrue(r.getRow().toString().contains(tableName.getNameAsString()))); + rs.forEach(r -> assertTrue(Bytes.toString(r.getRow()).contains(tableName.getNameAsString()))); } @Test @@ -138,16 +143,24 @@ public void testScanTableWithRegionReplicaId() throws IOException { scan.setReplicaId(RegionReplicaUtil.DEFAULT_REPLICA_ID); scan.setConsistency(Consistency.TIMELINE); ResultScanner rs = table.getScanner(scan); - rs.forEach(r -> assertEquals(ROW, r.getRow().toString())); + rs.forEach(r -> assertEquals(ROW, Bytes.toString(r.getRow()))); } @Test - public void testScanTableWithNonExistingRegionReplicaId() throws IOException { - Table table = connection.getTable(tableName); - Scan scan = new Scan(); - scan.setReplicaId(NON_EXISTING_REGION_REPLICA_ID); - scan.setConsistency(Consistency.TIMELINE); - exception.expect(DoNotRetryIOException.class); - table.getScanner(scan); + public void testScanTableWithNonExistingRegionReplicaId() + throws InterruptedException { + AsyncTableRegionLocator locator = asyncConn.getRegionLocator(tableName); + CompletableFuture future = locator.getRegionLocation(Bytes.toBytes(ROW), NON_EXISTING_REGION_REPLICA_ID, true); + try { + future.get(); + } catch (ExecutionException e) { + assertTrue(e.getCause() instanceof DoNotRetryIOException); + String message = "The specified region replica id '" + + NON_EXISTING_REGION_REPLICA_ID + "' does not exist, the REGION_REPLICATION of this " + + "table " + tableName.getNameAsString() + " is " + REGION_REPLICATION_COUNT + ", " + + "this means that the maximum region replica id you can specify is " + + (REGION_REPLICATION_COUNT - 1) + "."; + assertEquals(message, e.getCause().getMessage()); + } } } From eb976e84d61bb3f679c85550ba68f4969457aa65 Mon Sep 17 00:00:00 2001 From: guluo Date: Mon, 18 Mar 2024 20:13:01 +0800 Subject: [PATCH 4/6] splotless apply --- .../hbase/client/AsyncRegionLocator.java | 6 +- ...TableRegionLocatorWithRegionReplicaId.java | 60 ++++--- ...estScanTableWithReplicationFromClient.java | 166 ------------------ 3 files changed, 43 insertions(+), 189 deletions(-) delete mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScanTableWithReplicationFromClient.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java index 834fb1d030c9..88487d7684b4 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java @@ -200,9 +200,9 @@ CompletableFuture getRegionLocation(TableName tableName, byte[] future .completeExceptionally(new DoNotRetryIOException("The specified region replica id '" + replicaId + "' does not exist, the REGION_REPLICATION of this table " - + tableName.getNameAsString() + " is " + regionReplicationCount + " , this means " - + "that the maximum region replica id you can specify is " - + (regionReplicationCount - 1) + ".")); + + tableName.getNameAsString() + " is " + regionReplicationCount + "," + + " this means that the maximum region replica id you can specify is " + + (regionReplicationCount - 1) + ".")); return; } // meta region can not be split right now so we always call the same method. diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableRegionLocatorWithRegionReplicaId.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableRegionLocatorWithRegionReplicaId.java index ccdf253f12c5..875b953bcb2d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableRegionLocatorWithRegionReplicaId.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableRegionLocatorWithRegionReplicaId.java @@ -1,5 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.apache.hadoop.hbase.client; +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.CompletableFuture; +import java.util.concurrent.ExecutionException; import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtil; @@ -20,12 +44,6 @@ import org.junit.experimental.categories.Category; import org.junit.rules.ExpectedException; import org.junit.rules.TestName; -import java.util.List; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; @Category({ MediumTests.class, ClientTests.class }) public class TestAsyncTableRegionLocatorWithRegionReplicaId { @@ -100,24 +118,25 @@ public void tearDown() throws Exception { public void testMetaTableRegionLocatorWithRegionReplicaId() throws ExecutionException, InterruptedException { AsyncTableRegionLocator locator = asyncConn.getRegionLocator(TableName.META_TABLE_NAME); - CompletableFuture - future = locator.getRegionLocation(tableName.getName(), RegionReplicaUtil.DEFAULT_REPLICA_ID, true); + CompletableFuture future = + locator.getRegionLocation(tableName.getName(), RegionReplicaUtil.DEFAULT_REPLICA_ID, true); HRegionLocation hrl = future.get(); assertNotNull(hrl); } @Test - public void testMetaTableRegionLocatorWithNonExistingRegionReplicaId() throws InterruptedException { + public void testMetaTableRegionLocatorWithNonExistingRegionReplicaId() + throws InterruptedException { AsyncTableRegionLocator locator = asyncConn.getRegionLocator(TableName.META_TABLE_NAME); - CompletableFuture - future = locator.getRegionLocation(tableName.getName(), NON_EXISTING_REGION_REPLICA_ID, true); + CompletableFuture future = + locator.getRegionLocation(tableName.getName(), NON_EXISTING_REGION_REPLICA_ID, true); try { future.get(); } catch (ExecutionException e) { assertTrue(e.getCause() instanceof DoNotRetryIOException); - String message = "The specified region replica id '" - + NON_EXISTING_REGION_REPLICA_ID + "' does not exist, the REGION_REPLICATION of this " - + "table " + TableName.META_TABLE_NAME.getNameAsString() + " is " + String message = "The specified region replica id '" + NON_EXISTING_REGION_REPLICA_ID + + "' does not exist, the REGION_REPLICATION of this table " + + TableName.META_TABLE_NAME.getNameAsString() + " is " + TableDescriptorBuilder.DEFAULT_REGION_REPLICATION + ", " + "this means that the maximum region replica id you can specify is " + (TableDescriptorBuilder.DEFAULT_REGION_REPLICATION - 1) + "."; @@ -129,8 +148,8 @@ public void testMetaTableRegionLocatorWithNonExistingRegionReplicaId() throws In public void testTableRegionLocatorWithRegionReplicaId() throws ExecutionException, InterruptedException { AsyncTableRegionLocator locator = asyncConn.getRegionLocator(tableName); - CompletableFuture - future = locator.getRegionLocation(Bytes.toBytes(ROW), RegionReplicaUtil.DEFAULT_REPLICA_ID, true); + CompletableFuture future = + locator.getRegionLocation(Bytes.toBytes(ROW), RegionReplicaUtil.DEFAULT_REPLICA_ID, true); HRegionLocation hrl = future.get(); assertNotNull(hrl); } @@ -138,14 +157,15 @@ public void testTableRegionLocatorWithRegionReplicaId() @Test public void testTableRegionLocatorWithNonExistingRegionReplicaId() throws InterruptedException { AsyncTableRegionLocator locator = asyncConn.getRegionLocator(tableName); - CompletableFuture future = locator.getRegionLocation(Bytes.toBytes(ROW), NON_EXISTING_REGION_REPLICA_ID, true); + CompletableFuture future = + locator.getRegionLocation(Bytes.toBytes(ROW), NON_EXISTING_REGION_REPLICA_ID, true); try { future.get(); } catch (ExecutionException e) { assertTrue(e.getCause() instanceof DoNotRetryIOException); - String message = "The specified region replica id '" - + NON_EXISTING_REGION_REPLICA_ID + "' does not exist, the REGION_REPLICATION of this " - + "table " + tableName.getNameAsString() + " is " + REGION_REPLICATION_COUNT + ", " + String message = "The specified region replica id '" + NON_EXISTING_REGION_REPLICA_ID + + "' does not exist, the REGION_REPLICATION of this table " + tableName.getNameAsString() + + " is " + REGION_REPLICATION_COUNT + ", " + "this means that the maximum region replica id you can specify is " + (REGION_REPLICATION_COUNT - 1) + "."; assertEquals(message, e.getCause().getMessage()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScanTableWithReplicationFromClient.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScanTableWithReplicationFromClient.java deleted file mode 100644 index 188bee65f777..000000000000 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScanTableWithReplicationFromClient.java +++ /dev/null @@ -1,166 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.hadoop.hbase.client; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - -import java.io.IOException; -import java.util.List; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; -import org.apache.hadoop.hbase.DoNotRetryIOException; -import org.apache.hadoop.hbase.HBaseClassTestRule; -import org.apache.hadoop.hbase.HBaseTestingUtil; -import org.apache.hadoop.hbase.HRegionLocation; -import org.apache.hadoop.hbase.TableName; -import org.apache.hadoop.hbase.filter.PrefixFilter; -import org.apache.hadoop.hbase.regionserver.HRegion; -import org.apache.hadoop.hbase.testclassification.ClientTests; -import org.apache.hadoop.hbase.testclassification.MediumTests; -import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.io.IOUtils; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.ClassRule; -import org.junit.Rule; -import org.junit.Test; -import org.junit.experimental.categories.Category; -import org.junit.rules.ExpectedException; -import org.junit.rules.TestName; - -@Category({ MediumTests.class, ClientTests.class }) -public class TestScanTableWithReplicationFromClient { - @ClassRule - public static final HBaseClassTestRule CLASS_RULE = - HBaseClassTestRule.forClass(TestScanTableWithReplicationFromClient.class); - - @Rule - public TestName name = new TestName(); - - private ExpectedException exception = ExpectedException.none(); - - private final static HBaseTestingUtil UTIL = new HBaseTestingUtil(); - private static final String ROW = "r1"; - private static final byte[] FAMILY = Bytes.toBytes("info"); - private static final int REGION_REPLICATION_COUNT = 2; - // region replica id starts from 0 - private static final int NON_EXISTING_REGION_REPLICA_ID = REGION_REPLICATION_COUNT; - private static Connection connection; - private static AsyncConnection asyncConn; - private static Admin admin; - private TableName tableName; - - @BeforeClass - public static void setUpBeforeClass() throws Exception { - UTIL.startMiniCluster(1); - connection = UTIL.getConnection(); - asyncConn = ConnectionFactory.createAsyncConnection(UTIL.getConfiguration()).get(); - admin = UTIL.getAdmin(); - } - - @AfterClass - public static void tearDownAfterClass() throws Exception { - IOUtils.cleanup(null, admin); - IOUtils.cleanup(null, connection); - UTIL.shutdownMiniCluster(); - } - - @Before - public void setUp() throws Exception { - tableName = TableName.valueOf(name.getMethodName()); - ColumnFamilyDescriptor columnFamilyDescriptor = - ColumnFamilyDescriptorBuilder.newBuilder(FAMILY).build(); - TableDescriptor tableDescriptor = - TableDescriptorBuilder.newBuilder(tableName).setColumnFamily(columnFamilyDescriptor) - .setRegionReplication(REGION_REPLICATION_COUNT).build(); - admin.createTable(tableDescriptor); - UTIL.waitTableAvailable(tableName); - assertTrue(admin.tableExists(tableName)); - assertEquals(REGION_REPLICATION_COUNT, tableDescriptor.getRegionReplication()); - - List regions = UTIL.getHBaseCluster().getRegions(tableName); - assertEquals(REGION_REPLICATION_COUNT, regions.size()); - - Table table = connection.getTable(tableName); - Put put = new Put(Bytes.toBytes(ROW)).addColumn(FAMILY, Bytes.toBytes("q"), - Bytes.toBytes("test_value")); - table.put(put); - admin.flush(tableName); - - Scan scan = new Scan(); - ResultScanner rs = table.getScanner(scan); - rs.forEach(r -> assertEquals(ROW, Bytes.toString(r.getRow()))); - } - - @After - public void tearDown() throws Exception { - UTIL.deleteTable(tableName); - } - - @Test - public void testScanMetaWithRegionReplicaId() throws IOException { - Table metaTable = connection.getTable(TableName.META_TABLE_NAME); - Scan scan = new Scan(); - scan.setFilter(new PrefixFilter(tableName.getName())); - scan.setReplicaId(RegionReplicaUtil.DEFAULT_REPLICA_ID); - scan.setConsistency(Consistency.TIMELINE); - ResultScanner rs = metaTable.getScanner(scan); - rs.forEach(r -> assertTrue(Bytes.toString(r.getRow()).contains(tableName.getNameAsString()))); - } - - @Test - public void testScanMetaWithNonExistingRegionReplicaId() throws IOException { - Table metaTable = connection.getTable(TableName.META_TABLE_NAME); - Scan scan = new Scan(); - scan.setReplicaId(NON_EXISTING_REGION_REPLICA_ID); - scan.setConsistency(Consistency.TIMELINE); - exception.expect(DoNotRetryIOException.class); - metaTable.getScanner(scan); - } - - @Test - public void testScanTableWithRegionReplicaId() throws IOException { - Table table = connection.getTable(tableName); - Scan scan = new Scan(); - scan.setReplicaId(RegionReplicaUtil.DEFAULT_REPLICA_ID); - scan.setConsistency(Consistency.TIMELINE); - ResultScanner rs = table.getScanner(scan); - rs.forEach(r -> assertEquals(ROW, Bytes.toString(r.getRow()))); - } - - @Test - public void testScanTableWithNonExistingRegionReplicaId() - throws InterruptedException { - AsyncTableRegionLocator locator = asyncConn.getRegionLocator(tableName); - CompletableFuture future = locator.getRegionLocation(Bytes.toBytes(ROW), NON_EXISTING_REGION_REPLICA_ID, true); - try { - future.get(); - } catch (ExecutionException e) { - assertTrue(e.getCause() instanceof DoNotRetryIOException); - String message = "The specified region replica id '" - + NON_EXISTING_REGION_REPLICA_ID + "' does not exist, the REGION_REPLICATION of this " - + "table " + tableName.getNameAsString() + " is " + REGION_REPLICATION_COUNT + ", " - + "this means that the maximum region replica id you can specify is " - + (REGION_REPLICATION_COUNT - 1) + "."; - assertEquals(message, e.getCause().getMessage()); - } - } -} From b22f7d879d2163766c57f9e98c9cc0a977737727 Mon Sep 17 00:00:00 2001 From: guluo Date: Mon, 18 Mar 2024 20:39:08 +0800 Subject: [PATCH 5/6] update error message --- .../apache/hadoop/hbase/client/AsyncRegionLocator.java | 4 ++-- .../TestAsyncTableRegionLocatorWithRegionReplicaId.java | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java index 88487d7684b4..06b164492824 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java @@ -198,8 +198,8 @@ CompletableFuture getRegionLocation(TableName tableName, byte[] int regionReplicationCount = tdesc.getRegionReplication(); if (replicaId >= regionReplicationCount) { future - .completeExceptionally(new DoNotRetryIOException("The specified region replica id '" - + replicaId + "' does not exist, the REGION_REPLICATION of this table " + .completeExceptionally(new DoNotRetryIOException("The specified region replica id " + + replicaId + " does not exist, the REGION_REPLICATION of this table " + tableName.getNameAsString() + " is " + regionReplicationCount + "," + " this means that the maximum region replica id you can specify is " + (regionReplicationCount - 1) + ".")); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableRegionLocatorWithRegionReplicaId.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableRegionLocatorWithRegionReplicaId.java index 875b953bcb2d..2b82ef4a8d53 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableRegionLocatorWithRegionReplicaId.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableRegionLocatorWithRegionReplicaId.java @@ -134,8 +134,8 @@ public void testMetaTableRegionLocatorWithNonExistingRegionReplicaId() future.get(); } catch (ExecutionException e) { assertTrue(e.getCause() instanceof DoNotRetryIOException); - String message = "The specified region replica id '" + NON_EXISTING_REGION_REPLICA_ID - + "' does not exist, the REGION_REPLICATION of this table " + String message = "The specified region replica id " + NON_EXISTING_REGION_REPLICA_ID + + " does not exist, the REGION_REPLICATION of this table " + TableName.META_TABLE_NAME.getNameAsString() + " is " + TableDescriptorBuilder.DEFAULT_REGION_REPLICATION + ", " + "this means that the maximum region replica id you can specify is " @@ -163,8 +163,8 @@ public void testTableRegionLocatorWithNonExistingRegionReplicaId() throws Interr future.get(); } catch (ExecutionException e) { assertTrue(e.getCause() instanceof DoNotRetryIOException); - String message = "The specified region replica id '" + NON_EXISTING_REGION_REPLICA_ID - + "' does not exist, the REGION_REPLICATION of this table " + tableName.getNameAsString() + String message = "The specified region replica id " + NON_EXISTING_REGION_REPLICA_ID + + " does not exist, the REGION_REPLICATION of this table " + tableName.getNameAsString() + " is " + REGION_REPLICATION_COUNT + ", " + "this means that the maximum region replica id you can specify is " + (REGION_REPLICATION_COUNT - 1) + "."; From 8b5be6c668450f58786d13dccba54636cab6e556 Mon Sep 17 00:00:00 2001 From: guluo Date: Tue, 19 Mar 2024 22:33:09 +0800 Subject: [PATCH 6/6] Updating modifiers from public to private --- .../java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java index 06b164492824..e528e8b8f662 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java @@ -152,7 +152,7 @@ CompletableFuture getRegionLocations(TableName tableName, byte[ }, AsyncRegionLocator::getRegionNames, supplier); } - public void internalAddListener(CompletableFuture future, + private void internalAddListener(CompletableFuture future, CompletableFuture locsFuture, TableName tableName, byte[] row, int replicaId, RegionLocateType type) { addListener(locsFuture, (locs, error) -> {