HBASE-26790 getAllRegionLocations can cache locations with null hostname (#4575)
Signed-off-by: Andrew Purtell <apurtell@apache.org>
This commit is contained in:
parent
a9918c504b
commit
3c3927c60f
|
@ -61,9 +61,13 @@ class AsyncTableRegionLocatorImpl implements AsyncTableRegionLocator {
|
|||
.thenApply(locs -> Arrays.asList(locs.getRegionLocations()));
|
||||
}
|
||||
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)));
|
||||
.getTableHRegionLocations(conn.getTable(TableName.META_TABLE_NAME), tableName);
|
||||
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;
|
||||
}
|
||||
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
|
|
@ -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;
|
||||
|
@ -42,6 +43,7 @@ import org.apache.hadoop.conf.Configuration;
|
|||
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;
|
||||
|
@ -53,6 +55,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;
|
||||
|
@ -65,6 +68,8 @@ import org.junit.runners.Parameterized;
|
|||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
import org.apache.hbase.thirdparty.com.google.common.collect.Lists;
|
||||
|
||||
@Category({ MediumTests.class, ClientTests.class })
|
||||
@RunWith(Parameterized.class)
|
||||
public class TestAsyncNonMetaRegionLocator {
|
||||
|
@ -480,4 +485,66 @@ public class TestAsyncNonMetaRegionLocator {
|
|||
.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
|
||||
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);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -19,6 +19,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;
|
||||
|
||||
|
@ -28,15 +29,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 {
|
||||
|
@ -52,6 +61,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);
|
||||
|
@ -64,6 +76,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 =
|
||||
|
|
Loading…
Reference in New Issue