HBASE-21228 Memory leak since AbstractFSWAL caches Thread object and never 
clean later


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/9321f7d8
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/9321f7d8
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/9321f7d8

Branch: refs/heads/branch-1.3
Commit: 9321f7d86cb1eee4508d96f261189b90b85e714c
Parents: 10e4868
Author: Allan Yang <allan...@apache.org>
Authored: Thu Sep 27 16:44:46 2018 +0800
Committer: Andrew Purtell <apurt...@apache.org>
Committed: Thu Sep 27 17:51:43 2018 -0700

----------------------------------------------------------------------
 .../hadoop/hbase/regionserver/wal/FSHLog.java   | 26 +++++++++-----------
 1 file changed, 12 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/9321f7d8/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java
 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java
index f508bd7..eb734bb 100644
--- 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java
+++ 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java
@@ -190,12 +190,11 @@ public class FSHLog implements WAL {
   private final RingBufferEventHandler ringBufferEventHandler;
 
   /**
-   * Map of {@link SyncFuture}s keyed by Handler objects.  Used so we reuse 
SyncFutures.
-   * TODO: Reus FSWALEntry's rather than create them anew each time as we do 
SyncFutures here.
-   * TODO: Add a FSWalEntry and SyncFuture as thread locals on handlers rather 
than have them
-   * get them from this Map?
+   * Map of {@link SyncFuture}s owned by Thread objects. Used so we reuse 
SyncFutures.
+   * Thread local is used so JVM can GC the terminated thread for us. See 
HBASE-21228
+   * <p>
    */
-  private final Map<Thread, SyncFuture> syncFuturesByHandler;
+  private final ThreadLocal<SyncFuture> cachedSyncFutures;
 
   /**
    * The highest known outstanding unsync'd WALEdit sequence number where 
sequence number is the
@@ -569,8 +568,12 @@ public class FSHLog implements WAL {
         maxHandlersCount);
     this.disruptor.handleExceptionsWith(new RingBufferExceptionHandler());
     this.disruptor.handleEventsWith(new RingBufferEventHandler [] 
{this.ringBufferEventHandler});
-    // Presize our map of SyncFutures by handler objects.
-    this.syncFuturesByHandler = new ConcurrentHashMap<Thread, 
SyncFuture>(maxHandlersCount);
+    this.cachedSyncFutures = new ThreadLocal<SyncFuture>() {
+      @Override
+      protected SyncFuture initialValue() {
+        return new SyncFuture();
+      }
+    };
     // Starting up threads in constructor is a no no; Interface should have an 
init call.
     this.disruptor.start();
   }
@@ -1413,7 +1416,7 @@ public class FSHLog implements WAL {
       // SyncFuture reuse by thread, if TimeoutIOException happens, ringbuffer
       // still refer to it, so if this thread use it next time may get a wrong
       // result.
-      this.syncFuturesByHandler.remove(Thread.currentThread());
+      this.cachedSyncFutures.remove();
       throw tioe;
     } catch (InterruptedException ie) {
       LOG.warn("Interrupted", ie);
@@ -1431,12 +1434,7 @@ public class FSHLog implements WAL {
   }
 
   private SyncFuture getSyncFuture(final long sequence, Span span) {
-    SyncFuture syncFuture = 
this.syncFuturesByHandler.get(Thread.currentThread());
-    if (syncFuture == null) {
-      syncFuture = new SyncFuture();
-      this.syncFuturesByHandler.put(Thread.currentThread(), syncFuture);
-    }
-    return syncFuture.reset(sequence, span);
+    return cachedSyncFutures.get().reset(sequence);
   }
 
   private void postSync(final long timeInNanos, final int handlerSyncs) {

Reply via email to