You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by bb...@apache.org on 2022/06/24 11:52:27 UTC
[hbase] branch branch-2.4 updated: HBASE-26790 getAllRegionLocations can cache locations with null hostname (#4575)
This is an automated email from the ASF dual-hosted git repository.
bbeaudreault pushed a commit to branch branch-2.4
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2.4 by this push:
new ce3a37ebac2 HBASE-26790 getAllRegionLocations can cache locations with null hostname (#4575)
ce3a37ebac2 is described below
commit ce3a37ebac2c78c0c0c1847acda09421525e35ae
Author: Bryan Beaudreault <bb...@hubspot.com>
AuthorDate: Fri Jun 24 07:40:42 2022 -0400
HBASE-26790 getAllRegionLocations can cache locations with null hostname (#4575)
Signed-off-by: Andrew Purtell <ap...@apache.org>
---
.../hbase/client/AsyncTableRegionLocatorImpl.java | 8 ++-
.../apache/hadoop/hbase/client/HRegionLocator.java | 6 +-
.../client/TestAsyncNonMetaRegionLocator.java | 66 ++++++++++++++++++++++
.../hbase/client/TestRegionLocationCaching.java | 59 +++++++++++++++++++
4 files changed, 136 insertions(+), 3 deletions(-)
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTableRegionLocatorImpl.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTableRegionLocatorImpl.java
index 62f8de70ea8..8573bf6fafe 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTableRegionLocatorImpl.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTableRegionLocatorImpl.java
@@ -62,8 +62,12 @@ class AsyncTableRegionLocatorImpl implements AsyncTableRegionLocator {
}
CompletableFuture<List<HRegionLocation>> future = AsyncMetaTableAccessor
.getTableHRegionLocations(conn.getTable(TableName.META_TABLE_NAME), tableName);
- addListener(future, (locs, error) -> locs
- .forEach(loc -> conn.getLocator().getNonMetaRegionLocator().addLocationToCache(loc)));
+ addListener(future, (locs, error) -> locs.forEach(loc -> {
+ // the cache assumes that all locations have a serverName. only add if that's true
+ if (loc.getServerName() != null) {
+ conn.getLocator().getNonMetaRegionLocator().addLocationToCache(loc);
+ }
+ }));
return future;
}
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HRegionLocator.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HRegionLocator.java
index ce8fc3087f4..013a840af49 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HRegionLocator.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HRegionLocator.java
@@ -79,7 +79,11 @@ public class HRegionLocator implements RegionLocator {
for (HRegionLocation location : locations.getRegionLocations()) {
regions.add(location);
}
- connection.cacheLocation(tableName, locations);
+ RegionLocations cleaned = locations.removeElementsWithNullLocation();
+ // above can return null if all locations had null location
+ if (cleaned != null) {
+ connection.cacheLocation(tableName, cleaned);
+ }
}
return regions;
}
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncNonMetaRegionLocator.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncNonMetaRegionLocator.java
index 47e718566d2..f5a9d97264b 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncNonMetaRegionLocator.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncNonMetaRegionLocator.java
@@ -26,6 +26,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import java.io.IOException;
@@ -41,6 +42,7 @@ import org.apache.hadoop.hbase.CatalogReplicaMode;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseTestingUtility;
import org.apache.hadoop.hbase.HRegionLocation;
+import org.apache.hadoop.hbase.MetaTableAccessor;
import org.apache.hadoop.hbase.NotServingRegionException;
import org.apache.hadoop.hbase.RegionLocations;
import org.apache.hadoop.hbase.ServerName;
@@ -52,6 +54,7 @@ import org.apache.hadoop.hbase.security.User;
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.hbase.util.EnvironmentEdgeManager;
import org.apache.hadoop.hbase.util.ServerRegionReplicaUtil;
import org.junit.After;
import org.junit.AfterClass;
@@ -64,6 +67,7 @@ import org.junit.runners.Parameterized;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.apache.hbase.thirdparty.com.google.common.collect.Lists;
import org.apache.hbase.thirdparty.com.google.common.io.Closeables;
@Category({ MediumTests.class, ClientTests.class })
@@ -481,4 +485,66 @@ public class TestAsyncNonMetaRegionLocator {
region.getStartKey()));
}
}
+
+ @Test
+ public void testDoNotCacheLocationWithNullServerNameWhenGetAllLocations() throws Exception {
+ createMultiRegionTable();
+ AsyncConnectionImpl conn = (AsyncConnectionImpl) ConnectionFactory
+ .createAsyncConnection(TEST_UTIL.getConfiguration()).get();
+ List<RegionInfo> regions = TEST_UTIL.getAdmin().getRegions(TABLE_NAME);
+ RegionInfo chosen = regions.get(0);
+
+ // re-populate region cache
+ AsyncTableRegionLocator regionLocator = conn.getRegionLocator(TABLE_NAME);
+ regionLocator.clearRegionLocationCache();
+ regionLocator.getAllRegionLocations().get();
+
+ // expect all to be non-null at first
+ int tries = 3;
+ checkRegionsWithRetries(conn, regions, null, tries);
+
+ // clear servername from region info
+ Put put = MetaTableAccessor.makePutFromRegionInfo(chosen, EnvironmentEdgeManager.currentTime());
+ MetaTableAccessor.addEmptyLocation(put, 0);
+ MetaTableAccessor.putsToMetaTable(TEST_UTIL.getConnection(), Lists.newArrayList(put));
+
+ // re-populate region cache again. check that we succeeded in nulling the servername
+ regionLocator.clearRegionLocationCache();
+ for (HRegionLocation loc : regionLocator.getAllRegionLocations().get()) {
+ if (loc.getRegion().equals(chosen)) {
+ assertNull(loc.getServerName());
+ }
+ }
+
+ // expect all but chosen to be non-null. chosen should be null because serverName was null
+ checkRegionsWithRetries(conn, regions, chosen, tries);
+ }
+
+ // caching of getAllRegionLocations is async. so we give it a couple tries
+ private void checkRegionsWithRetries(AsyncConnectionImpl conn, List<RegionInfo> regions,
+ RegionInfo chosen, int retries) throws InterruptedException {
+ while (true) {
+ try {
+ checkRegions(conn, regions, chosen);
+ break;
+ } catch (AssertionError e) {
+ if (retries-- <= 0) {
+ throw e;
+ }
+ Thread.sleep(500);
+ }
+ }
+ }
+
+ private void checkRegions(AsyncConnectionImpl conn, List<RegionInfo> regions, RegionInfo chosen) {
+ for (RegionInfo region : regions) {
+ RegionLocations fromCache = conn.getLocator().getNonMetaRegionLocator()
+ .getRegionLocationInCache(TABLE_NAME, region.getStartKey());
+ if (region.equals(chosen)) {
+ assertNull(fromCache);
+ } else {
+ assertNotNull(fromCache);
+ }
+ }
+ }
}
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRegionLocationCaching.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRegionLocationCaching.java
index 4dbb39e2dbe..ea94123d618 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRegionLocationCaching.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRegionLocationCaching.java
@@ -18,6 +18,7 @@
package org.apache.hadoop.hbase.client;
import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
@@ -26,15 +27,23 @@ import java.util.ArrayList;
import java.util.List;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HRegionLocation;
+import org.apache.hadoop.hbase.MetaTableAccessor;
+import org.apache.hadoop.hbase.RegionLocations;
import org.apache.hadoop.hbase.TableName;
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.hbase.util.EnvironmentEdgeManager;
import org.junit.AfterClass;
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.TestName;
+
+import org.apache.hbase.thirdparty.com.google.common.collect.Lists;
@Category({ MediumTests.class, ClientTests.class })
public class TestRegionLocationCaching {
@@ -50,6 +59,9 @@ public class TestRegionLocationCaching {
private static byte[] FAMILY = Bytes.toBytes("testFamily");
private static byte[] QUALIFIER = Bytes.toBytes("testQualifier");
+ @Rule
+ public final TestName name = new TestName();
+
@BeforeClass
public static void setUpBeforeClass() throws Exception {
TEST_UTIL.startMiniCluster(SLAVES);
@@ -62,6 +74,53 @@ public class TestRegionLocationCaching {
TEST_UTIL.shutdownMiniCluster();
}
+ @Test
+ public void testDoNotCacheLocationWithNullServerNameWhenGetAllLocations() throws Exception {
+ TableName tableName = TableName.valueOf(name.getMethodName());
+ TEST_UTIL.createTable(tableName, new byte[][] { FAMILY });
+ TEST_UTIL.waitUntilAllRegionsAssigned(tableName);
+
+ ConnectionImplementation conn = (ConnectionImplementation) TEST_UTIL.getConnection();
+ List<RegionInfo> regions = TEST_UTIL.getAdmin().getRegions(tableName);
+ RegionInfo chosen = regions.get(0);
+
+ // re-populate region cache
+ RegionLocator regionLocator = TEST_UTIL.getConnection().getRegionLocator(tableName);
+ regionLocator.clearRegionLocationCache();
+ regionLocator.getAllRegionLocations();
+
+ // expect all to be non-null at first
+ checkRegions(tableName, conn, regions, null);
+
+ // clear servername from region info
+ Put put = MetaTableAccessor.makePutFromRegionInfo(chosen, EnvironmentEdgeManager.currentTime());
+ MetaTableAccessor.addEmptyLocation(put, 0);
+ MetaTableAccessor.putsToMetaTable(TEST_UTIL.getConnection(), Lists.newArrayList(put));
+
+ // re-populate region cache again. check that we succeeded in nulling the servername
+ regionLocator.clearRegionLocationCache();
+ for (HRegionLocation loc : regionLocator.getAllRegionLocations()) {
+ if (loc.getRegion().equals(chosen)) {
+ assertNull(loc.getServerName());
+ }
+ }
+
+ // expect all but chosen to be non-null. chosen should be null because serverName was null
+ checkRegions(tableName, conn, regions, chosen);
+ }
+
+ private void checkRegions(TableName tableName, ConnectionImplementation conn,
+ List<RegionInfo> regions, RegionInfo chosen) {
+ for (RegionInfo region : regions) {
+ RegionLocations fromCache = conn.getCachedLocation(tableName, region.getStartKey());
+ if (region.equals(chosen)) {
+ assertNull(fromCache);
+ } else {
+ assertNotNull(fromCache);
+ }
+ }
+ }
+
@Test
public void testCachingForHTableMultiplexerSinglePut() throws Exception {
HTableMultiplexer multiplexer =