From c468ec7f9e74a7bbd57ac85e8a5bab45bed813ad Mon Sep 17 00:00:00 2001 From: Andrew Purtell Date: Wed, 14 Nov 2018 15:14:52 -0800 Subject: [PATCH] HBASE-21464 Splitting blocked with meta NSRE during split transaction Ignore row key when looking up meta table locations in the MetaCache When looking up the locations of hbase:meta with useCache false, clear all previous cache entries for it first Fix Table reference leaks in MetaTableAccessor with try-with-resources --- .../hadoop/hbase/MetaTableAccessor.java | 95 ++++++++----------- .../hbase/client/ConnectionManager.java | 19 +++- .../apache/hadoop/hbase/client/MetaCache.java | 14 +++ 3 files changed, 72 insertions(+), 56 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java index 440f8c6646..fd7a97b392 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java @@ -175,8 +175,7 @@ public class MetaTableAccessor { * @return An {@link Table} for hbase:meta * @throws IOException */ - static Table getMetaHTable(final Connection connection) - throws IOException { + static Table getMetaHTable(final Connection connection) throws IOException { // We used to pass whole CatalogTracker in here, now we just pass in Connection if (connection == null) { throw new NullPointerException("No connection"); @@ -248,11 +247,13 @@ public class MetaTableAccessor { } Get get = new Get(row); get.addFamily(HConstants.CATALOG_FAMILY); - Result r = get(getMetaHTable(connection), get); - RegionLocations locations = getRegionLocations(r); - return locations == null - ? null - : locations.getRegionLocation(parsedInfo == null ? 0 : parsedInfo.getReplicaId()); + try (Table metaTable = getMetaHTable(connection)) { + Result r = get(metaTable, get); + RegionLocations locations = getRegionLocations(r); + return locations == null + ? null + : locations.getRegionLocation(parsedInfo == null ? 0 : parsedInfo.getReplicaId()); + } } /** @@ -267,8 +268,10 @@ public class MetaTableAccessor { byte[] row = getMetaKeyForRegion(regionInfo); Get get = new Get(row); get.addFamily(HConstants.CATALOG_FAMILY); - Result r = get(getMetaHTable(connection), get); - return getRegionLocation(r, regionInfo, regionInfo.getReplicaId()); + try (Table metaTable = getMetaHTable(connection)) { + Result r = get(metaTable, get); + return getRegionLocation(r, regionInfo, regionInfo.getReplicaId()); + } } /** Returns the row key to use for this regionInfo */ @@ -300,7 +303,9 @@ public class MetaTableAccessor { byte[] regionName) throws IOException { Get get = new Get(regionName); get.addFamily(HConstants.CATALOG_FAMILY); - return get(getMetaHTable(connection), get); + try (Table metaTable = getMetaHTable(connection)) { + return get(metaTable, get); + } } /** @@ -631,19 +636,19 @@ public class MetaTableAccessor { scan.setCaching(caching); } scan.addFamily(HConstants.CATALOG_FAMILY); - Table metaTable = getMetaHTable(connection); - ResultScanner scanner = null; - try { - scanner = metaTable.getScanner(scan); - Result data; - while((data = scanner.next()) != null) { - if (data.isEmpty()) continue; - // Break if visit returns false. - if (!visitor.visit(data)) break; + try (Table metaTable = getMetaHTable(connection)) { + try (ResultScanner scanner = metaTable.getScanner(scan)) { + Result data; + while ((data = scanner.next()) != null) { + if (data.isEmpty()) { + continue; + } + // Break if visit returns false. + if (!visitor.visit(data)) { + break; + } + } } - } finally { - if (scanner != null) scanner.close(); - metaTable.close(); } } @@ -1020,7 +1025,9 @@ public class MetaTableAccessor { */ static void putToMetaTable(final Connection connection, final Put p) throws IOException { - put(getMetaHTable(connection), p); + try (Table metaTable = getMetaHTable(connection)) { + put(metaTable, p); + } } /** @@ -1044,11 +1051,8 @@ public class MetaTableAccessor { */ public static void putsToMetaTable(final Connection connection, final List ps) throws IOException { - Table t = getMetaHTable(connection); - try { - t.put(ps); - } finally { - t.close(); + try (Table metaTable = getMetaHTable(connection)) { + metaTable.put(ps); } } @@ -1073,11 +1077,8 @@ public class MetaTableAccessor { */ public static void deleteFromMetaTable(final Connection connection, final List deletes) throws IOException { - Table t = getMetaHTable(connection); - try { - t.delete(deletes); - } finally { - t.close(); + try (Table metaTable = getMetaHTable(connection)) { + metaTable.delete(deletes); } } @@ -1116,15 +1117,12 @@ public class MetaTableAccessor { public static void mutateMetaTable(final Connection connection, final List mutations) throws IOException { - Table t = getMetaHTable(connection); - try { - t.batch(mutations); + try (Table metaTable = getMetaHTable(connection)) { + metaTable.batch(mutations); } catch (InterruptedException e) { InterruptedIOException ie = new InterruptedIOException(e.getMessage()); ie.initCause(e); throw ie; - } finally { - t.close(); } } @@ -1188,11 +1186,8 @@ public class MetaTableAccessor { */ public static void addRegionToMeta(Connection connection, HRegionInfo regionInfo, HRegionInfo splitA, HRegionInfo splitB) throws IOException { - Table meta = getMetaHTable(connection); - try { - addRegionToMeta(meta, regionInfo, splitA, splitB); - } finally { - meta.close(); + try (Table metaTable = getMetaHTable(connection)) { + addRegionToMeta(metaTable, regionInfo, splitA, splitB); } } @@ -1269,8 +1264,7 @@ public class MetaTableAccessor { HRegionInfo regionA, HRegionInfo regionB, ServerName sn, int regionReplication, long masterSystemTime) throws IOException { - Table meta = getMetaHTable(connection); - try { + try (Table metaTable = getMetaHTable(connection)) { HRegionInfo copyOfMerged = new HRegionInfo(mergedRegion); // use the maximum of what master passed us vs local time. @@ -1298,9 +1292,7 @@ public class MetaTableAccessor { byte[] tableRow = Bytes.toBytes(mergedRegion.getRegionNameAsString() + HConstants.DELIMITER); - multiMutate(meta, tableRow, putOfMerged, deleteA, deleteB); - } finally { - meta.close(); + multiMutate(metaTable, tableRow, putOfMerged, deleteA, deleteB); } } @@ -1318,8 +1310,7 @@ public class MetaTableAccessor { public static void splitRegion(final Connection connection, HRegionInfo parent, HRegionInfo splitA, HRegionInfo splitB, ServerName sn, int regionReplication) throws IOException { - Table meta = getMetaHTable(connection); - try { + try (Table metaTable = getMetaHTable(connection)) { HRegionInfo copyOfParent = new HRegionInfo(parent); copyOfParent.setOffline(true); copyOfParent.setSplit(true); @@ -1343,9 +1334,7 @@ public class MetaTableAccessor { } byte[] tableRow = Bytes.toBytes(parent.getRegionNameAsString() + HConstants.DELIMITER); - multiMutate(meta, tableRow, putParent, putA, putB); - } finally { - meta.close(); + multiMutate(metaTable, tableRow, putParent, putA, putB); } } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java index 7cf09c2702..e243c542a1 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java @@ -1236,10 +1236,9 @@ class ConnectionManager { // HBASE-10785: We cache the location of the META itself, so that we are not overloading // zookeeper with one request for every region lookup. We cache the META with empty row // key in MetaCache. - byte[] metaCacheKey = HConstants.EMPTY_START_ROW; // use byte[0] as the row for meta RegionLocations locations = null; if (useCache) { - locations = getCachedLocation(tableName, metaCacheKey); + locations = getCachedLocation(tableName); if (locations != null && locations.getRegionLocation(replicaId) != null) { return locations; } @@ -1250,10 +1249,12 @@ class ConnectionManager { // Check the cache again for a hit in case some other thread made the // same query while we were waiting on the lock. if (useCache) { - locations = getCachedLocation(tableName, metaCacheKey); + locations = getCachedLocation(tableName); if (locations != null && locations.getRegionLocation(replicaId) != null) { return locations; } + } else { + clearRegionCache(tableName); } // Look up from zookeeper @@ -1432,6 +1433,18 @@ class ConnectionManager { metaCache.cacheLocation(tableName, location); } + /** + * Search the cache for a location that fits our table + * Return null if no suitable region is located. + * + * @param tableName + * @param row + * @return Null or region location found in cache. + */ + RegionLocations getCachedLocation(final TableName tableName) { + return metaCache.getCachedLocation(tableName); + } + /** * Search the cache for a location that fits our table and row key. * Return null if no suitable region is located. diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetaCache.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetaCache.java index 2a172e7f81..c51b34680b 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetaCache.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetaCache.java @@ -66,6 +66,20 @@ public class MetaCache { this.metrics = metrics; } + /** + * Search the cache for a location that fits our table + * Return null if no suitable region is located. + * + * @return Null or region location found in cache. + */ + public RegionLocations getCachedLocation(final TableName tableName) { + RegionLocations locations = new RegionLocations(); + for (RegionLocations r: getTableLocations(tableName).values()) { + locations.mergeLocations(r); + } + return locations; + } + /** * Search the cache for a location that fits our table and row key. * Return null if no suitable region is located. -- 2.19.0