From b581b322a10b23dcd35a91334a5d3aedd3b1611a Mon Sep 17 00:00:00 2001 From: sambong0113 <36813712+sambong0113@users.noreply.github.com> Date: Sat, 22 May 2021 10:55:15 +0900 Subject: [PATCH] HBASE-25817 Memory leak from thrift server hashMap (#3257) Use GuavaCache in thrift server hashmap Signed-off-by: Michael Stack --- .../thrift/ThriftHBaseServiceHandler.java | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftHBaseServiceHandler.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftHBaseServiceHandler.java index e38ac6cab82..e964d934646 100644 --- a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftHBaseServiceHandler.java +++ b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftHBaseServiceHandler.java @@ -19,6 +19,8 @@ package org.apache.hadoop.hbase.thrift; +import static org.apache.hadoop.hbase.HConstants.DEFAULT_HBASE_CLIENT_SCANNER_TIMEOUT_PERIOD; +import static org.apache.hadoop.hbase.HConstants.HBASE_CLIENT_SCANNER_TIMEOUT_PERIOD; import static org.apache.hadoop.hbase.thrift.Constants.COALESCE_INC_KEY; import static org.apache.hadoop.hbase.util.Bytes.getBytes; @@ -26,10 +28,10 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.TreeMap; +import java.util.concurrent.TimeUnit; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.CatalogFamilyFormat; import org.apache.hadoop.hbase.Cell; @@ -79,6 +81,8 @@ import org.apache.hadoop.hbase.thrift.generated.TRowResult; import org.apache.hadoop.hbase.thrift.generated.TScan; import org.apache.hadoop.hbase.thrift.generated.TThriftServerType; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hbase.thirdparty.com.google.common.cache.Cache; +import org.apache.hbase.thirdparty.com.google.common.cache.CacheBuilder; import org.apache.thrift.TException; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; @@ -99,7 +103,7 @@ public class ThriftHBaseServiceHandler extends HBaseServiceHandler implements Hb // nextScannerId and scannerMap are used to manage scanner state private int nextScannerId = 0; - private HashMap scannerMap; + private Cache scannerMap; IncrementCoalescer coalescer; /** @@ -137,7 +141,7 @@ public class ThriftHBaseServiceHandler extends HBaseServiceHandler implements Hb * @return a Scanner, or null if ID was invalid. */ private synchronized ResultScannerWrapper getScanner(int id) { - return scannerMap.get(id); + return scannerMap.getIfPresent(id); } /** @@ -147,14 +151,19 @@ public class ThriftHBaseServiceHandler extends HBaseServiceHandler implements Hb * @param id the ID of the scanner to remove * @return a Scanner, or null if ID was invalid. */ - private synchronized ResultScannerWrapper removeScanner(int id) { - return scannerMap.remove(id); + private synchronized void removeScanner(int id) { + scannerMap.invalidate(id); } protected ThriftHBaseServiceHandler(final Configuration c, final UserProvider userProvider) throws IOException { super(c, userProvider); - scannerMap = new HashMap<>(); + long cacheTimeout = c.getLong(HBASE_CLIENT_SCANNER_TIMEOUT_PERIOD, DEFAULT_HBASE_CLIENT_SCANNER_TIMEOUT_PERIOD) * 2; + + scannerMap = CacheBuilder.newBuilder() + .expireAfterAccess(cacheTimeout, TimeUnit.MILLISECONDS) + .build(); + this.coalescer = new IncrementCoalescer(this); } @@ -858,6 +867,10 @@ public class ThriftHBaseServiceHandler extends HBaseServiceHandler implements Hb } catch (IOException e) { LOG.warn(e.getMessage(), e); throw getIOError(e); + } finally { + // Add scanner back to scannerMap; protects against case + // where scanner expired during processing of request. + scannerMap.put(id, resultScannerWrapper); } return ThriftUtilities.rowResultFromHBase(results, resultScannerWrapper.isColumnSorted()); }