Repository: hbase Updated Branches: refs/heads/branch-1 ede5c6326 -> 319a9d3ff
HBASE-12148 Remove TimeRangeTracker as point of contention when many threads writing a Store Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/ccd16050 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/ccd16050 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/ccd16050 Branch: refs/heads/branch-1 Commit: ccd160501f44831a3ef89d94f15db6f1fe3196c1 Parents: ede5c63 Author: stack <st...@apache.org> Authored: Thu Oct 2 12:40:55 2014 -0700 Committer: stack <st...@apache.org> Committed: Thu Oct 2 12:40:55 2014 -0700 ---------------------------------------------------------------------- .../hbase/regionserver/TimeRangeTracker.java | 85 ++++++++++---------- 1 file changed, 43 insertions(+), 42 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/ccd16050/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 6f25129..0044634 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 @@ -22,13 +22,10 @@ import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; -import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellUtil; -import org.apache.hadoop.hbase.KeyValue; -import org.apache.hadoop.hbase.KeyValue.Type; +import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.io.TimeRange; -import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.io.Writable; /** @@ -40,30 +37,41 @@ import org.apache.hadoop.io.Writable; */ @InterfaceAudience.Private public class TimeRangeTracker implements Writable { - - long minimumTimestamp = -1; + static final long INITIAL_MINIMUM_TIMESTAMP = Long.MAX_VALUE; + long minimumTimestamp = INITIAL_MINIMUM_TIMESTAMP; 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) { - this.minimumTimestamp = trt.getMinimumTimestamp(); - this.maximumTimestamp = trt.getMaximumTimestamp(); + set(trt.getMinimumTimestamp(), trt.getMaximumTimestamp()); } public TimeRangeTracker(long minimumTimestamp, long maximumTimestamp) { - this.minimumTimestamp = minimumTimestamp; - this.maximumTimestamp = 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; } /** @@ -80,36 +88,30 @@ 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 */ - 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; + 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; + } + } + } } - return; } /** @@ -118,8 +120,7 @@ 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()); } /** @@ -150,4 +151,4 @@ public class TimeRangeTracker implements Writable { public synchronized String toString() { return "[" + minimumTimestamp + "," + maximumTimestamp + "]"; } -} +} \ No newline at end of file