[hbase] branch branch-2.4 updated: HBASE-25817 Memory leak from thrift server hashMap (#3257)

[zhangduo]

This is an automated email from the ASF dual-hosted git repository.

zhangduo pushed a commit to branch branch-2.4
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-2.4 by this push:
     new 0e18900  HBASE-25817 Memory leak from thrift server hashMap (#3257)
0e18900 is described below

commit 0e18900c36ba051b0b829564ad2982f204eee68f
Author: sambong0113 <36813712+sambong0...@users.noreply.github.com>
AuthorDate: Sat May 22 10:55:15 2021 +0900

    HBASE-25817 Memory leak from thrift server hashMap (#3257)
    
    Use GuavaCache in thrift server hashmap
    
    Signed-off-by: Michael Stack <st...@apache.org>
---
 .../hbase/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 973a03d..ff9e4b5 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<Integer, ResultScannerWrapper> scannerMap;
+  private Cache<Integer, ResultScannerWrapper> 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());
   }