[2/5] git commit: Revert "HBASE-12148 Remove TimeRangeTracker as point of contention when many threads writing a Store"

[ap...@apache.org]

Revert "HBASE-12148 Remove TimeRangeTracker as point of contention when many threads writing a Store"

This reverts commit 9f2e748f0a33a5b939a3768c8aeb10432ad43b23.

Post commit consensus is this change isn't ready yet.


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

Branch: refs/heads/0.98
Commit: 00d3eec5d69a8ac735a23fc09be7b8a13c85c0cb
Parents: be64e48
Author: Andrew Purtell <ap...@apache.org>
Authored: Wed Oct 8 12:00:52 2014 -0700
Committer: Andrew Purtell <ap...@apache.org>
Committed: Wed Oct 8 12:00:52 2014 -0700

----------------------------------------------------------------------
 .../hbase/regionserver/TimeRangeTracker.java    | 80 ++++++++++----------
 1 file changed, 39 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/00d3eec5/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/TimeRangeTracker.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/TimeRangeTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/TimeRangeTracker.java
index 3d48b7a..dfa0ded 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/TimeRangeTracker.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/TimeRangeTracker.java
@@ -24,7 +24,9 @@ import java.io.IOException;
 
 import org.apache.hadoop.hbase.classification.InterfaceAudience;
 import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.KeyValue.Type;
 import org.apache.hadoop.hbase.io.TimeRange;
+import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.io.Writable;
 
 /**
@@ -36,41 +38,30 @@ import org.apache.hadoop.io.Writable;
  */
 @InterfaceAudience.Private
 public class TimeRangeTracker implements Writable {
-  static final long INITIAL_MINIMUM_TIMESTAMP = Long.MAX_VALUE;
-  long minimumTimestamp = INITIAL_MINIMUM_TIMESTAMP;
+
+  long minimumTimestamp = -1;
   long maximumTimestamp = -1;
 
   /**
    * Default constructor.
    * Initializes TimeRange to be null
    */
-  public TimeRangeTracker() {}
+  public TimeRangeTracker() {
+
+  }
 
   /**
    * Copy Constructor
    * @param trt source TimeRangeTracker
    */
   public TimeRangeTracker(final TimeRangeTracker trt) {
-    set(trt.getMinimumTimestamp(), trt.getMaximumTimestamp());
+    this.minimumTimestamp = trt.getMinimumTimestamp();
+    this.maximumTimestamp = trt.getMaximumTimestamp();
   }
 
   public TimeRangeTracker(long minimumTimestamp, long maximumTimestamp) {
-    set(minimumTimestamp, maximumTimestamp);
-  }
-
-  private void set(final long min, final long max) {
-    this.minimumTimestamp = min;
-    this.maximumTimestamp = max;
-  }
-
-  /**
-   * @param l
-   * @return True if we initialized values
-   */
-  private boolean init(final long l) {
-    if (this.minimumTimestamp != INITIAL_MINIMUM_TIMESTAMP) return false;
-    set(l, l);
-    return true;
+    this.minimumTimestamp = minimumTimestamp;
+    this.maximumTimestamp = maximumTimestamp;
   }
 
   /**
@@ -87,30 +78,36 @@ public class TimeRangeTracker implements Writable {
   }
 
   /**
+   * Update the current TimestampRange to include the timestamp from Key.
+   * If the Key is of type DeleteColumn or DeleteFamily, it includes the
+   * entire time range from 0 to timestamp of the key.
+   * @param key
+   */
+  public void includeTimestamp(final byte[] key) {
+    includeTimestamp(Bytes.toLong(key,key.length-KeyValue.TIMESTAMP_TYPE_SIZE));
+    int type = key[key.length - 1];
+    if (type == Type.DeleteColumn.getCode() ||
+        type == Type.DeleteFamily.getCode()) {
+      includeTimestamp(0);
+    }
+  }
+
+  /**
    * If required, update the current TimestampRange to include timestamp
    * @param timestamp the timestamp value to include
    */
-  void includeTimestamp(final long timestamp) {
-    // Do test outside of synchronization block.  Synchronization in here can be problematic
-    // when many threads writing one Store -- they can all pile up trying to add in here.
-    // Happens when doing big write upload where we are hammering on one region.
-    if (timestamp < this.minimumTimestamp) {
-      synchronized (this) {
-        if (!init(timestamp)) {
-          if (timestamp < this.minimumTimestamp) {
-            this.minimumTimestamp = timestamp;
-          }
-        }
-      }
-    } else if (timestamp > this.maximumTimestamp) {
-      synchronized (this) {
-        if (!init(timestamp)) {
-          if (this.maximumTimestamp < timestamp) {
-            this.maximumTimestamp =  timestamp;
-          }
-        }
-      }
+  private synchronized void includeTimestamp(final long timestamp) {
+    if (maximumTimestamp == -1) {
+      minimumTimestamp = timestamp;
+      maximumTimestamp = timestamp;
+    }
+    else if (minimumTimestamp > timestamp) {
+      minimumTimestamp = timestamp;
+    }
+    else if (maximumTimestamp < timestamp) {
+      maximumTimestamp = timestamp;
     }
+    return;
   }
 
   /**
@@ -119,7 +116,8 @@ public class TimeRangeTracker implements Writable {
    * @return True if there is overlap, false otherwise
    */
   public synchronized boolean includesTimeRange(final TimeRange tr) {
-    return (this.minimumTimestamp < tr.getMax() && this.maximumTimestamp >= tr.getMin());
+    return (this.minimumTimestamp < tr.getMax() &&
+        this.maximumTimestamp >= tr.getMin());
   }
 
   /**