You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by nd...@apache.org on 2014/03/20 00:59:45 UTC

svn commit: r1579475 - in /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal: FSHLog.java RingBufferTruck.java

Author: ndimiduk
Date: Wed Mar 19 23:59:44 2014
New Revision: 1579475

URL: http://svn.apache.org/r1579475
Log:
HBASE-10792 RingBufferTruck does not release its payload

Modified:
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/RingBufferTruck.java

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java?rev=1579475&r1=1579474&r2=1579475&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java Wed Mar 19 23:59:44 2014
@@ -1779,15 +1779,15 @@ class FSHLog implements HLog, Syncable {
 
       // TODO: Trace only working for appends, not for syncs.
       TraceScope scope =
-        truck.getSpanPayload() != null? Trace.continueSpan(truck.getSpanPayload()): null;
+        truck.hasSpanPayload() ? Trace.continueSpan(truck.unloadSpanPayload()) : null;
       try {
-        if (truck.getSyncFuturePayload() != null) {
-          this.syncFutures[this.syncFuturesCount++] = truck.getSyncFuturePayload();
+        if (truck.hasSyncFuturePayload()) {
+          this.syncFutures[this.syncFuturesCount++] = truck.unloadSyncFuturePayload();
           // Force flush of syncs if we are carrying a full complement of syncFutures.
           if (this.syncFuturesCount == this.syncFutures.length) endOfBatch = true;
-        } else if (truck.getFSWALEntryPayload() != null) {
+        } else if (truck.hasFSWALEntryPayload()) {
           try {
-            append(truck.getFSWALEntryPayload());
+            append(truck.unloadFSWALEntryPayload());
           } catch (Exception e) {
             // If append fails, presume any pending syncs will fail too; let all waiting handlers
             // know of the exception.

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/RingBufferTruck.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/RingBufferTruck.java?rev=1579475&r1=1579474&r2=1579475&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/RingBufferTruck.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/RingBufferTruck.java Wed Mar 19 23:59:44 2014
@@ -26,7 +26,9 @@ import com.lmax.disruptor.EventFactory;
 /**
  * A 'truck' to carry a payload across the {@link FSHLog} ring buffer from Handler to WAL.
  * Has EITHER a {@link FSWALEntry} for making an append OR it has a {@link SyncFuture} to
- * represent a 'sync' invocation. Immutable but instances get recycled on the ringbuffer.
+ * represent a 'sync' invocation. Truck instances are reused by the disruptor when it gets
+ * around to it so their payload references must be discarded on consumption to release them
+ * to GC.
  */
 @InterfaceAudience.Private
 class RingBufferTruck {
@@ -42,28 +44,73 @@ class RingBufferTruck {
    */
   private Span span;
 
+  /**
+   * Load the truck with a {@link FSWALEntry} and associated {@link Span}.
+   */
   void loadPayload(final FSWALEntry entry, final Span span) {
     this.entry = entry;
     this.span = span;
     this.syncFuture = null;
   }
 
+  /**
+   * Load the truck with a {@link SyncFuture}.
+   */
   void loadPayload(final SyncFuture syncFuture) {
     this.syncFuture = syncFuture;
     this.entry = null;
     this.span = null;
   }
 
-  FSWALEntry getFSWALEntryPayload() {
-    return this.entry;
+  /**
+   * return {@code true} when this truck is carrying a {@link FSWALEntry},
+   * {@code false} otherwise.
+   */
+  boolean hasFSWALEntryPayload() {
+    return this.entry != null;
   }
 
-  SyncFuture getSyncFuturePayload() {
-    return this.syncFuture;
+  /**
+   * return {@code true} when this truck is carrying a {@link SyncFuture},
+   * {@code false} otherwise.
+   */
+  boolean hasSyncFuturePayload() {
+    return this.syncFuture != null;
+  }
+
+  /**
+   * return {@code true} when this truck is carrying a {@link Span},
+   * {@code false} otherwise.
+   */
+  boolean hasSpanPayload() {
+    return this.span != null;
   }
 
-  Span getSpanPayload() {
-    return this.span;
+  /**
+   * Unload the truck of its {@link FSWALEntry} payload. The internal refernce is released.
+   */
+  FSWALEntry unloadFSWALEntryPayload() {
+    FSWALEntry ret = this.entry;
+    this.entry = null;
+    return ret;
+  }
+
+  /**
+   * Unload the truck of its {@link SyncFuture} payload. The internal refernce is released.
+   */
+  SyncFuture unloadSyncFuturePayload() {
+    SyncFuture ret = this.syncFuture;
+    this.syncFuture = null;
+    return ret;
+  }
+
+  /**
+   * Unload the truck of its {@link Span} payload. The internal refernce is released.
+   */
+  Span unloadSpanPayload() {
+    Span ret = this.span;
+    this.span = null;
+    return ret;
   }
 
   /**