diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ClassSize.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ClassSize.java index 3dce955..9d4f47c 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ClassSize.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ClassSize.java @@ -65,6 +65,9 @@ public class ClassSize { /** String overhead */ public static final int STRING; + /** RANGE_TIMESTAMP overhead */ + public static final int RANGE_TIMESTAMP; + /** Overhead for TreeMap */ public static final int TREEMAP; @@ -192,7 +195,9 @@ public class ClassSize { TIMERANGE = align(ClassSize.OBJECT + Bytes.SIZEOF_LONG * 2 + Bytes.SIZEOF_BOOLEAN); - TIMERANGE_TRACKER = align(ClassSize.OBJECT + Bytes.SIZEOF_LONG * 2); + RANGE_TIMESTAMP = align(ClassSize.OBJECT + Bytes.SIZEOF_LONG * 2); + + TIMERANGE_TRACKER = align(ClassSize.OBJECT + REFERENCE); CELL_SKIPLIST_SET = align(OBJECT + REFERENCE); 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 beadde6..ee7bfa5 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 @@ -21,12 +21,13 @@ package org.apache.hadoop.hbase.regionserver; import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; - import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellUtil; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.io.TimeRange; import org.apache.hadoop.io.Writable; +import java.util.concurrent.atomic.AtomicReference; + /** * Stores the minimum and maximum timestamp values (both are inclusive). @@ -36,10 +37,11 @@ import org.apache.hadoop.io.Writable; * at read time to match against the required TimeRange. */ @InterfaceAudience.Private +// TODO: Undo Writable here. public class TimeRangeTracker implements Writable { static final long INITIAL_MINIMUM_TIMESTAMP = Long.MAX_VALUE; - long minimumTimestamp = INITIAL_MINIMUM_TIMESTAMP; - long maximumTimestamp = -1; + private final AtomicReference timeRange = + new AtomicReference<>(new TimeRangeTimestamp(INITIAL_MINIMUM_TIMESTAMP,-1)); /** * Default constructor. @@ -52,26 +54,16 @@ public class TimeRangeTracker implements Writable { * @param trt source TimeRangeTracker */ public TimeRangeTracker(final TimeRangeTracker trt) { - set(trt.getMinimumTimestamp(), trt.getMaximumTimestamp()); + // Here we clone the TimeRangeTimestamp from the passed in trt. + set(new TimeRangeTimestamp(trt.timeRange.get())); } public TimeRangeTracker(long minimumTimestamp, long maximumTimestamp) { - set(minimumTimestamp, maximumTimestamp); - } - - private void set(final long min, final long max) { - this.minimumTimestamp = min; - this.maximumTimestamp = max; + set(new TimeRangeTimestamp(minimumTimestamp, maximumTimestamp)); } - /** - * @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; + private void set(final TimeRangeTimestamp rts) { + this.timeRange.set(rts); } /** @@ -94,25 +86,17 @@ public class TimeRangeTracker implements Writable { @edu.umd.cs.findbugs.annotations.SuppressWarnings(value="MT_CORRECTNESS", justification="Intentional") 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; - } + while (true) { + TimeRangeTimestamp rt = timeRange.get(); + if (rt.isMaxTs(timestamp) || rt.isMinTs(timestamp)) { + if (!timeRange.compareAndSet(rt, + new TimeRangeTimestamp(rt.isMinTs(timestamp)? timestamp: rt.minimumTimestamp, + rt.isMaxTs(timestamp)? timestamp: rt.maximumTimestamp))) { + // CAS failed. Go around again. + continue; } } + break; } } @@ -121,36 +105,68 @@ public class TimeRangeTracker implements Writable { * @param tr TimeRange * @return True if there is overlap, false otherwise */ - public synchronized boolean includesTimeRange(final TimeRange tr) { - return (this.minimumTimestamp < tr.getMax() && this.maximumTimestamp >= tr.getMin()); + public boolean includesTimeRange(final TimeRange tr) { + TimeRangeTimestamp rt = this.timeRange.get(); + return (rt.minimumTimestamp < tr.getMax() && rt.maximumTimestamp >= tr.getMin()); } /** * @return the minimumTimestamp */ - public synchronized long getMinimumTimestamp() { - return minimumTimestamp; + public long getMinimumTimestamp() { + return this.timeRange.get().minimumTimestamp; } /** * @return the maximumTimestamp */ - public synchronized long getMaximumTimestamp() { - return maximumTimestamp; + public long getMaximumTimestamp() { + return timeRange.get().maximumTimestamp; } - public synchronized void write(final DataOutput out) throws IOException { - out.writeLong(minimumTimestamp); - out.writeLong(maximumTimestamp); + public void write(final DataOutput out) throws IOException { + TimeRangeTimestamp rt = this.timeRange.get(); + out.writeLong(rt.minimumTimestamp); + out.writeLong(rt.maximumTimestamp); } - public synchronized void readFields(final DataInput in) throws IOException { - this.minimumTimestamp = in.readLong(); - this.maximumTimestamp = in.readLong(); + public void readFields(final DataInput in) throws IOException { + set(new TimeRangeTimestamp(in.readLong(),in.readLong())); } @Override - public synchronized String toString() { - return "[" + minimumTimestamp + "," + maximumTimestamp + "]"; + public String toString() { + return timeRange.get().toString(); + } + + /** + * Immutable datastructure with TimeRange min and max timestamps. + */ + private static class TimeRangeTimestamp { + private final long minimumTimestamp; + private final long maximumTimestamp; + + private TimeRangeTimestamp(TimeRangeTimestamp rts) { + this(rts.minimumTimestamp, rts.maximumTimestamp); + } + + private TimeRangeTimestamp(long minimumTimestamp, long maximumTimestamp) { + this.minimumTimestamp = minimumTimestamp; + this.maximumTimestamp = maximumTimestamp; + } + + boolean isMaxTs(long timestamp) { + return maximumTimestamp < timestamp; + } + + boolean isMinTs(long timestamp) { + return minimumTimestamp > timestamp; + } + + @Override + public String toString() { + return "[ mininumTimestamp=" + minimumTimestamp + + ", maximumTimestamp=" + maximumTimestamp + "]"; + } } } \ No newline at end of file diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MockStoreFile.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MockStoreFile.java index 32dc227..b4b94ec 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MockStoreFile.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MockStoreFile.java @@ -143,7 +143,7 @@ public class MockStoreFile extends StoreFile { @Override public long getMaxTimestamp() { - return timeRange == null ? Long.MAX_VALUE : timeRange.maximumTimestamp; + return timeRange == null ? Long.MAX_VALUE : timeRange.getMaximumTimestamp(); } @Override