Repository: hbase Updated Branches: refs/heads/branch-2.0 b4818fffd -> 493ca10e6
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/493ca10e Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/493ca10e Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/493ca10e Branch: refs/heads/branch-2.0 Commit: 493ca10e6a911221f072f15a52ecd373c59ea316 Parents: b4818ff Author: Allan Yang <allan...@apache.org> Authored: Thu Sep 27 15:00:30 2018 +0800 Committer: Allan Yang <allan...@apache.org> Committed: Thu Sep 27 15:00:30 2018 +0800 ---------------------------------------------------------------------- .../hbase/regionserver/wal/AbstractFSWAL.java | 27 ++++++++------------ 1 file changed, 11 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/493ca10e/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AbstractFSWAL.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AbstractFSWAL.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AbstractFSWAL.java index 9b31834..b493fd6 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AbstractFSWAL.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AbstractFSWAL.java @@ -34,8 +34,6 @@ import java.util.List; import java.util.Map; import java.util.OptionalLong; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ConcurrentNavigableMap; import java.util.concurrent.ConcurrentSkipListMap; import java.util.concurrent.CopyOnWriteArrayList; @@ -62,7 +60,6 @@ import org.apache.hadoop.hbase.log.HBaseMarkers; import org.apache.hadoop.hbase.regionserver.MultiVersionConcurrencyControl; import org.apache.hadoop.hbase.trace.TraceUtil; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.util.CollectionUtils; import org.apache.hadoop.hbase.util.CommonFSUtils; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.FSUtils; @@ -268,14 +265,11 @@ public abstract class AbstractFSWAL<W extends WriterBase> implements WAL { new ConcurrentSkipListMap<>(LOG_NAME_COMPARATOR); /** - * Map of {@link SyncFuture}s keyed by Handler objects. Used so we reuse SyncFutures. + * 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> - * TODO: Reuse FSWALEntry's rather than create them anew each time as we do SyncFutures here. - * <p> - * TODO: Add a FSWalEntry and SyncFuture as thread locals on handlers rather than have them get - * them from this Map? */ - private final ConcurrentMap<Thread, SyncFuture> syncFuturesByHandler; + private final ThreadLocal<SyncFuture> cachedSyncFutures; /** * The class name of the runtime implementation, used as prefix for logging/tracing. @@ -429,9 +423,12 @@ public abstract class AbstractFSWAL<W extends WriterBase> implements WAL { .toNanos(conf.getInt("hbase.regionserver.hlog.slowsync.ms", DEFAULT_SLOW_SYNC_TIME_MS)); this.walSyncTimeoutNs = TimeUnit.MILLISECONDS .toNanos(conf.getLong("hbase.regionserver.hlog.sync.timeout", DEFAULT_WAL_SYNC_TIMEOUT_MS)); - int maxHandlersCount = conf.getInt(HConstants.REGION_SERVER_HANDLER_COUNT, 200); - // Presize our map of SyncFutures by handler objects. - this.syncFuturesByHandler = new ConcurrentHashMap<>(maxHandlersCount); + this.cachedSyncFutures = new ThreadLocal<SyncFuture>() { + @Override + protected SyncFuture initialValue() { + return new SyncFuture(); + } + }; this.implClassName = getClass().getSimpleName(); } @@ -723,7 +720,7 @@ public abstract class AbstractFSWAL<W extends WriterBase> 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); @@ -873,9 +870,7 @@ public abstract class AbstractFSWAL<W extends WriterBase> implements WAL { } protected final SyncFuture getSyncFuture(long sequence) { - return CollectionUtils - .computeIfAbsent(syncFuturesByHandler, Thread.currentThread(), SyncFuture::new) - .reset(sequence); + return cachedSyncFutures.get().reset(sequence); } protected final void requestLogRoll(boolean tooFewReplicas) {
