From 0e18900c36ba051b0b829564ad2982f204eee68f 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 | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 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 973a03d1517..ff9e4b55134 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,11 +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.Cell; import org.apache.hadoop.hbase.CellBuilder; @@ -85,6 +86,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hbase.thirdparty.com.google.common.base.Throwables; +import org.apache.hbase.thirdparty.com.google.common.cache.Cache; +import org.apache.hbase.thirdparty.com.google.common.cache.CacheBuilder; /** * The HBaseServiceHandler is a glue object that connects Thrift RPC calls to the @@ -99,7 +102,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; /** @@ -141,7 +144,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); } /** @@ -151,14 +154,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); } @@ -863,6 +871,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()); }