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 0044634..50f1b72 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,6 +21,7 @@ package org.apache.hadoop.hbase.regionserver; import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; +import java.util.concurrent.atomic.AtomicLong; import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellUtil; @@ -38,8 +39,8 @@ 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; + final AtomicLong minimumTimestamp = new AtomicLong(INITIAL_MINIMUM_TIMESTAMP); + final AtomicLong maximumTimestamp = new AtomicLong(-1L); /** * Default constructor. @@ -60,8 +61,8 @@ public class TimeRangeTracker implements Writable { } private void set(final long min, final long max) { - this.minimumTimestamp = min; - this.maximumTimestamp = max; + this.minimumTimestamp.set(min); + this.maximumTimestamp.set(max); } /** @@ -69,7 +70,7 @@ public class TimeRangeTracker implements Writable { * @return True if we initialized values */ private boolean init(final long l) { - if (this.minimumTimestamp != INITIAL_MINIMUM_TIMESTAMP) return false; + if (getMinimumTimestamp() != INITIAL_MINIMUM_TIMESTAMP) return false; set(l, l); return true; } @@ -92,26 +93,47 @@ public class TimeRangeTracker implements Writable { * @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; - } - } + if (!setMinimum(timestamp)) { + setMaximum(timestamp); + } + } + + private boolean setMinimum(final long timestamp) { + boolean result = false; + while (true) { + long current = getMinimumTimestamp(); + if (timestamp >= current) { + break; + } + if (init(timestamp)) { + result = true; + break; + } + if (this.minimumTimestamp.compareAndSet(current, timestamp)) { + result = true; + break; + } + } + return result; + } + + private boolean setMaximum(final long timestamp) { + boolean result = false; + while (true) { + long current = getMaximumTimestamp(); + if (timestamp <= current) { + break; + } + if (init(timestamp)) { + result = true; + break; } - } else if (timestamp > this.maximumTimestamp) { - synchronized (this) { - if (!init(timestamp)) { - if (this.maximumTimestamp < timestamp) { - this.maximumTimestamp = timestamp; - } - } + if (this.maximumTimestamp.compareAndSet(current, timestamp)) { + result = true; + break; } } + return result; } /** @@ -119,36 +141,35 @@ 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) { + return getMinimumTimestamp() < tr.getMax() && getMaximumTimestamp() >= tr.getMin(); } /** * @return the minimumTimestamp */ - public synchronized long getMinimumTimestamp() { - return minimumTimestamp; + public long getMinimumTimestamp() { + return minimumTimestamp.get(); } /** * @return the maximumTimestamp */ - public synchronized long getMaximumTimestamp() { - return maximumTimestamp; + public long getMaximumTimestamp() { + return maximumTimestamp.get(); } - public synchronized void write(final DataOutput out) throws IOException { - out.writeLong(minimumTimestamp); - out.writeLong(maximumTimestamp); + public void write(final DataOutput out) throws IOException { + out.writeLong(getMinimumTimestamp()); + out.writeLong(getMaximumTimestamp()); } - 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(in.readLong(), in.readLong()); } @Override - public synchronized String toString() { - return "[" + minimumTimestamp + "," + maximumTimestamp + "]"; + public String toString() { + return "[" + getMinimumTimestamp() + "," + getMaximumTimestamp() + "]"; } } \ No newline at end of file diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTimeRangeTracker.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTimeRangeTracker.java index edec023..da4e083 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTimeRangeTracker.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTimeRangeTracker.java @@ -24,6 +24,7 @@ import org.apache.hadoop.hbase.testclassification.SmallTests; import org.apache.hadoop.hbase.testclassification.RegionServerTests; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.mortbay.log.Log; @Category({RegionServerTests.class, SmallTests.class}) public class TestTimeRangeTracker { @@ -89,14 +90,21 @@ public class TestTimeRangeTracker { public static void main(String[] args) throws InterruptedException { long start = System.currentTimeMillis(); final TimeRangeTracker trr = new TimeRangeTracker(); - final int threadCount = 5; - final int calls = 1024 * 1024 * 128; + final int threadCount = 32; + final int calls = 1024 * 1024 * 1024; Thread [] threads = new Thread[threadCount]; for (int i = 0; i < threads.length; i++) { Thread t = new Thread("" + i) { @Override public void run() { - for (int i = 0; i < calls; i++) trr.includeTimestamp(i); + try { + for (int i = 0; i < calls; i++) { + trr.includeTimestamp(i); + // Log.info(getName() + "" + i); + } + } catch (Throwable t) { + Log.info("", t); + } } }; t.start();