HBASE-26790 getAllRegionLocations can cache locations with null hostn…

…ame (#4565) Signed-off-by: Andrew Purtell <apurtell@apache.org>
apache · Jun 23, 2022 · 50f1115 · 50f1115
1 parent 342183e
commit 50f1115
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 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
@@ -64,8 +64,12 @@ public CompletableFuture<List<HRegionLocation>> getAllRegionLocations() {
       }
       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
@@ -26,6 +26,7 @@
 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.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.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.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 void testCacheLocationWhenGetAllLocations() throws Exception {
       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);
+      }
+    }
+  }
 }