From 42324094260e859e407ca47edcba3ad06340249e Mon Sep 17 00:00:00 2001 From: Walter Koetke Date: Thu, 17 Mar 2016 16:19:23 -0700 Subject: [PATCH] HBASE-12148: need to make TimeRangeTracker non blocking Summary: remove synchronization code and use AtomicReference instead Test Plan: unit tests --- .../org/apache/hadoop/hbase/util/ClassSize.java | 7 +- .../hbase/regionserver/TimeRangeTracker.java | 104 +++++++++++++-------- .../hadoop/hbase/regionserver/MockStoreFile.java | 2 +- 3 files changed, 70 insertions(+), 43 deletions(-) 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 77acf9b..6787004 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; @@ -190,7 +193,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..615e056 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). @@ -38,21 +39,23 @@ 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 maximumTimestamp = -1; + protected AtomicReference timeRange; /** * Default constructor. * Initializes TimeRange to be null */ - public TimeRangeTracker() {} + public TimeRangeTracker() { + timeRange = new AtomicReference<>(new RangeTimestamp(INITIAL_MINIMUM_TIMESTAMP,-1)); + } /** * Copy Constructor * @param trt source TimeRangeTracker */ public TimeRangeTracker(final TimeRangeTracker trt) { - set(trt.getMinimumTimestamp(), trt.getMaximumTimestamp()); + RangeTimestamp rt = trt.timeRange.get(); + set(rt.minimumTimestamp, rt.maximumTimestamp); } public TimeRangeTracker(long minimumTimestamp, long maximumTimestamp) { @@ -60,17 +63,17 @@ public class TimeRangeTracker implements Writable { } private void set(final long min, final long max) { - this.minimumTimestamp = min; - this.maximumTimestamp = max; + timeRange = new AtomicReference(new RangeTimestamp(min,max)); } /** - * @param l + * @param l the initial value * @return True if we initialized values */ private boolean init(final long l) { - if (this.minimumTimestamp != INITIAL_MINIMUM_TIMESTAMP) return false; - set(l, l); + RangeTimestamp trt = timeRange.get(); + if (trt.minimumTimestamp != INITIAL_MINIMUM_TIMESTAMP) return false; + set(1,1); return true; } @@ -94,26 +97,19 @@ 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; + while (true) { + RangeTimestamp rt = timeRange.get(); + if (rt.isMaxTs(timestamp) || rt.isMinTs(timestamp)) { + if (timeRange.compareAndSet(rt, + new RangeTimestamp(rt.isMinTs(timestamp) ? timestamp : rt.minimumTimestamp, + rt.isMaxTs(timestamp) ? timestamp : rt.maximumTimestamp))) { + return; // CAS Succeeded } + // else CAS Miss, Spin + } else { + return; // No need to modify } } - } else if (timestamp > this.maximumTimestamp) { - synchronized (this) { - if (!init(timestamp)) { - if (this.maximumTimestamp < timestamp) { - this.maximumTimestamp = timestamp; - } - } - } - } } /** @@ -121,36 +117,62 @@ 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) { + RangeTimestamp 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 { + RangeTimestamp 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 { + this.timeRange = new AtomicReference( + new RangeTimestamp(in.readLong(),in.readLong())); } @Override - public synchronized String toString() { - return "[" + minimumTimestamp + "," + maximumTimestamp + "]"; + public String toString() { + return timeRange.get().toString(); + } + + public static class RangeTimestamp { + long minimumTimestamp = INITIAL_MINIMUM_TIMESTAMP; + long maximumTimestamp = -1; + public RangeTimestamp() {}; + + public RangeTimestamp(long minimumTimestamp, long maximumTimestamp) { + this.minimumTimestamp = minimumTimestamp; + this.maximumTimestamp = maximumTimestamp; + } + + public boolean isMaxTs(long timestamp) { + return maximumTimestamp < timestamp; + } + + public boolean isMinTs(long timestamp) { + return minimumTimestamp > timestamp; + } + @Override + public String toString() { + return "[" + minimumTimestamp + "," + 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 621f1c2..e0c189b 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 @@ -125,7 +125,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 -- 2.3.2 (Apple Git-55)