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 06c39f6593e3..2f49186aeae8 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 @@ public CompletableFuture> getAllRegionLocations() { } CompletableFuture> 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; }, getClass().getSimpleName() + ".getAllRegionLocations"); } 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 c92358ec5232..7dd59c909617 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 @@ -101,7 +101,11 @@ public List getAllRegionLocations() throws IOException { 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; }, HRegionLocator::getRegionNames, supplier); 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 137e5f18b9ce..24e1ade54fd8 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.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.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.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,66 @@ public void testCacheLocationWhenGetAllLocations() throws Exception { region.getStartKey())); } } + + @Test + public void testDoNotCacheLocationWithNullServerNameWhenGetAllLocations() throws Exception { + createMultiRegionTable(); + AsyncConnectionImpl conn = (AsyncConnectionImpl) ConnectionFactory + .createAsyncConnection(TEST_UTIL.getConfiguration()).get(); + List 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 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 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 4dbb39e2dbe2..ea94123d6185 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.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 static void tearDownAfterClass() throws Exception { 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 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 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 =