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/23 12:24:56 UTC
[hbase] branch master updated: HBASE-26790 getAllRegionLocations can cache locations with null hostname (#4565)
This is an automated email from the ASF dual-hosted git repository.
bbeaudreault pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/master by this push:
new 50f11151fda HBASE-26790 getAllRegionLocations can cache locations with null hostname (#4565)
50f11151fda is described below
commit 50f11151fdabcc4b328b96117db374bddefec40f
Author: Bryan Beaudreault <bb...@hubspot.com>
AuthorDate: Thu Jun 23 08:24:49 2022 -0400
HBASE-26790 getAllRegionLocations can cache locations with null hostname (#4565)
Signed-off-by: Andrew Purtell <ap...@apache.org>
---
.../hbase/client/AsyncTableRegionLocatorImpl.java | 8 +++-
.../client/TestAsyncNonMetaRegionLocator.java | 49 ++++++++++++++++++++++
2 files changed, 55 insertions(+), 2 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 fd04e662dd7..b7ec7fcd872 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
@@ -64,8 +64,12 @@ class AsyncTableRegionLocatorImpl implements AsyncTableRegionLocator {
}
CompletableFuture<List<HRegionLocation>> future = ClientMetaTableAccessor
.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;
}, getClass().getSimpleName() + ".getAllRegionLocations");
}
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 f5f3cc4c6fb..6655bba3d4f 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.HBaseTestingUtil;
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.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameter;
+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 })
@@ -476,4 +480,49 @@ public class TestAsyncNonMetaRegionLocator {
assertNotNull(conn.getLocator().getRegionLocationInCache(TABLE_NAME, 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
+ checkRegions(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().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
+ checkRegions(conn, regions, chosen);
+ }
+
+ private void checkRegions(AsyncConnectionImpl conn, List<RegionInfo> regions, RegionInfo chosen) {
+ for (RegionInfo region : regions) {
+ RegionLocations fromCache =
+ conn.getLocator().getRegionLocationInCache(TABLE_NAME, region.getStartKey());
+ if (region.equals(chosen)) {
+ assertNull(fromCache);
+ } else {
+ assertNotNull(fromCache);
+ }
+ }
+ }
}