HBASE-15650 Remove TimeRangeTracker as point of contention when many threads
reading a StoreFile
Refactor so we use the immutable, unsynchronized TimeRange when doing
time-based checks at read time rather than use heavily synchronized
TimeRangeTracker; let TimeRangeTracker be for write-time only.
While in here, changed the Segment stuff so that when an immutable
segment, it uses TimeRange rather than TimeRangeTracker too.
M hbase-common/src/main/java/org/apache/hadoop/hbase/io/TimeRange.java
Make allTime final.
Add a includesTimeRange method copied from TimeRangeTracker.
M
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/TimeRangeTracker.java
Change name of a few methods so they match TimeRange methods that do
same thing.
(getTimeRangeTracker, getTimeRange, toTimeRange) add utility methods
M hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
Change Reader to use TimeRange-based checks instead of
TimeRangeTracker.
Conflicts:
hbase-common/src/main/java/org/apache/hadoop/hbase/io/TimeRange.java
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/TimeRangeTracker.java
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MockStoreFile.java
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTimeRangeTracker.java
Amending-Author: Andrew Purtell <[email protected]>
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/a0bb623c
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/a0bb623c
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/a0bb623c
Branch: refs/heads/0.98
Commit: a0bb623ccf805794c38a76b23d23507353108326
Parents: 3425696
Author: stack <[email protected]>
Authored: Thu Aug 4 13:10:40 2016 -0700
Committer: Andrew Purtell <[email protected]>
Committed: Thu Aug 4 13:10:40 2016 -0700
----------------------------------------------------------------------
.../java/org/apache/hadoop/hbase/CellUtil.java | 13 ++
.../org/apache/hadoop/hbase/io/TimeRange.java | 41 +++--
.../hbase/io/hfile/HFilePrettyPrinter.java | 3 +-
.../hadoop/hbase/regionserver/MemStore.java | 11 +-
.../hadoop/hbase/regionserver/StoreFile.java | 63 ++++----
.../hbase/regionserver/StoreFileScanner.java | 2 +-
.../hbase/regionserver/TimeRangeTracker.java | 155 +++++++++++++------
.../hbase/mapreduce/TestHFileOutputFormat.java | 7 +-
.../hbase/mapreduce/TestHFileOutputFormat2.java | 8 +-
.../hbase/regionserver/MockStoreFile.java | 24 ++-
.../regionserver/TestTimeRangeTracker.java | 56 ++++++-
11 files changed, 254 insertions(+), 129 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/a0bb623c/hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
b/hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
index cdfc72b..4268bfd 100644
--- a/hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
+++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
@@ -382,6 +382,19 @@ public final class CellUtil {
return cell.getTypeByte() == Type.DeleteFamily.getCode();
}
+ public static boolean isDeleteFamilyVersion(final Cell cell) {
+ return cell.getTypeByte() == Type.DeleteFamilyVersion.getCode();
+ }
+
+ /**
+ *
+ * @return True if this cell is a delete family or column type.
+ */
+ public static boolean isDeleteColumnOrFamily(Cell cell) {
+ int t = cell.getTypeByte();
+ return t == Type.DeleteColumn.getCode() || t ==
Type.DeleteFamily.getCode();
+ }
+
/**
* @param cell
* @return Estimate of the <code>cell</code> size in bytes.
http://git-wip-us.apache.org/repos/asf/hbase/blob/a0bb623c/hbase-common/src/main/java/org/apache/hadoop/hbase/io/TimeRange.java
----------------------------------------------------------------------
diff --git
a/hbase-common/src/main/java/org/apache/hadoop/hbase/io/TimeRange.java
b/hbase-common/src/main/java/org/apache/hadoop/hbase/io/TimeRange.java
index b2c3ebe..8bbc0eb 100644
--- a/hbase-common/src/main/java/org/apache/hadoop/hbase/io/TimeRange.java
+++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/io/TimeRange.java
@@ -36,11 +36,13 @@ import org.apache.hadoop.hbase.util.Bytes;
@InterfaceAudience.Public
@InterfaceStability.Stable
public class TimeRange {
- private static final long MIN_TIME = 0L;
- private static final long MAX_TIME = Long.MAX_VALUE;
+ public static final long INITIAL_MIN_TIMESTAMP = 0L;
+ private static final long MIN_TIME = INITIAL_MIN_TIMESTAMP;
+ public static final long INITIAL_MAX_TIMESTAMP = Long.MAX_VALUE;
+ static final long MAX_TIME = INITIAL_MAX_TIMESTAMP;
private long minStamp = MIN_TIME;
private long maxStamp = MAX_TIME;
- private boolean allTime = false;
+ private final boolean allTime;
/**
* Default constructor.
@@ -56,9 +58,7 @@ public class TimeRange {
*/
public TimeRange(long minStamp) {
this.minStamp = minStamp;
- if (this.minStamp == MIN_TIME){
- this.allTime = true;
- }
+ this.allTime = this.minStamp == MIN_TIME;
}
/**
@@ -67,6 +67,7 @@ public class TimeRange {
*/
public TimeRange(byte [] minStamp) {
this.minStamp = Bytes.toLong(minStamp);
+ this.allTime = false;
}
/**
@@ -81,14 +82,12 @@ public class TimeRange {
throw new IllegalArgumentException("Timestamp cannot be negative.
minStamp:" + minStamp
+ ", maxStamp" + maxStamp);
}
- if(maxStamp < minStamp) {
+ if (maxStamp < minStamp) {
throw new IOException("maxStamp is smaller than minStamp");
}
this.minStamp = minStamp;
this.maxStamp = maxStamp;
- if (this.minStamp == MIN_TIME && this.maxStamp == MAX_TIME){
- this.allTime = true;
- }
+ this.allTime = this.minStamp == MIN_TIME && this.maxStamp == MAX_TIME;
}
/**
@@ -147,9 +146,25 @@ public class TimeRange {
* @return true if within TimeRange, false if not
*/
public boolean withinTimeRange(long timestamp) {
- if(allTime) return true;
- // check if >= minStamp
- return (minStamp <= timestamp && timestamp < maxStamp);
+ if (this.allTime) {
+ return true;
+ }
+ // check if >= minStamp
+ return (minStamp <= timestamp && timestamp < maxStamp);
+ }
+
+ /**
+ * Check if the range has any overlap with TimeRange
+ * @param tr TimeRange
+ * @return True if there is overlap, false otherwise
+ */
+ // This method came from TimeRangeTracker. We used to go there for this
function but better
+ // to come here to the immutable, unsynchronized datastructure at read time.
+ public boolean includesTimeRange(final TimeRange tr) {
+ if (this.allTime) {
+ return true;
+ }
+ return getMin() < tr.getMax() && getMax() >= tr.getMin();
}
/**
http://git-wip-us.apache.org/repos/asf/hbase/blob/a0bb623c/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java
----------------------------------------------------------------------
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java
index 070d5d4..270b552 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java
@@ -417,8 +417,7 @@ public class HFilePrettyPrinter extends Configured
implements Tool {
TimeRangeTracker timeRangeTracker = new TimeRangeTracker();
Writables.copyWritable(e.getValue(), timeRangeTracker);
- out.println(timeRangeTracker.getMinimumTimestamp() + "...."
- + timeRangeTracker.getMaximumTimestamp());
+ out.println(timeRangeTracker.getMin() + "...." +
timeRangeTracker.getMax());
} else if (Bytes.compareTo(e.getKey(), FileInfo.AVG_KEY_LEN) == 0
|| Bytes.compareTo(e.getKey(), FileInfo.AVG_VALUE_LEN) == 0) {
out.println(Bytes.toInt(e.getValue()));
http://git-wip-us.apache.org/repos/asf/hbase/blob/a0bb623c/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java
----------------------------------------------------------------------
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java
index 9c63fc5..09be5a5 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java
@@ -41,6 +41,7 @@ import org.apache.hadoop.hbase.KeyValue;
import org.apache.hadoop.hbase.KeyValueUtil;
import org.apache.hadoop.hbase.client.Scan;
import org.apache.hadoop.hbase.io.HeapSize;
+import org.apache.hadoop.hbase.io.TimeRange;
import org.apache.hadoop.hbase.regionserver.MemStoreLAB.Allocation;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.util.ClassSize;
@@ -680,11 +681,11 @@ public class MemStore implements HeapSize {
* @return False if the key definitely does not exist in this Memstore
*/
public boolean shouldSeek(Scan scan, long oldestUnexpiredTS) {
- return (timeRangeTracker.includesTimeRange(scan.getTimeRange()) ||
- snapshotTimeRangeTracker.includesTimeRange(scan.getTimeRange()))
- && (Math.max(timeRangeTracker.getMaximumTimestamp(),
- snapshotTimeRangeTracker.getMaximumTimestamp()) >=
- oldestUnexpiredTS);
+ TimeRange timeRange = scan.getTimeRange();
+ return (timeRangeTracker.includesTimeRange(timeRange) ||
+ snapshotTimeRangeTracker.includesTimeRange(timeRange)) &&
+ (Math.max(timeRangeTracker.getMax(), snapshotTimeRangeTracker.getMax())
+ >= oldestUnexpiredTS);
}
public TimeRangeTracker getSnapshotTimeRangeTracker() {
http://git-wip-us.apache.org/repos/asf/hbase/blob/a0bb623c/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
----------------------------------------------------------------------
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
index b959152..417ca14 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
@@ -48,6 +48,7 @@ import org.apache.hadoop.hbase.KeyValue.KVComparator;
import org.apache.hadoop.hbase.classification.InterfaceAudience;
import org.apache.hadoop.hbase.client.Scan;
import org.apache.hadoop.hbase.io.FSDataInputStreamWrapper;
+import org.apache.hadoop.hbase.io.TimeRange;
import org.apache.hadoop.hbase.io.encoding.DataBlockEncoding;
import org.apache.hadoop.hbase.io.hfile.BlockType;
import org.apache.hadoop.hbase.io.hfile.CacheConfig;
@@ -61,7 +62,6 @@ import org.apache.hadoop.hbase.util.BloomFilterFactory;
import org.apache.hadoop.hbase.util.BloomFilterWriter;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.util.ChecksumType;
-import org.apache.hadoop.hbase.util.Writables;
import org.apache.hadoop.io.WritableUtils;
/**
@@ -468,15 +468,11 @@ public class StoreFile {
reader.loadBloomfilter(BlockType.DELETE_FAMILY_BLOOM_META);
try {
- byte [] timerangeBytes = metadataMap.get(TIMERANGE_KEY);
- if (timerangeBytes != null) {
- this.reader.timeRangeTracker = new TimeRangeTracker();
- Writables.copyWritable(timerangeBytes, this.reader.timeRangeTracker);
- }
+ this.reader.timeRange =
TimeRangeTracker.getTimeRange(metadataMap.get(TIMERANGE_KEY));
} catch (IllegalArgumentException e) {
LOG.error("Error reading timestamp range data from meta -- " +
"proceeding without", e);
- this.reader.timeRangeTracker = null;
+ this.reader.timeRange = null;
}
return this.reader;
}
@@ -496,7 +492,7 @@ public class StoreFile {
} catch (IOException e) {
try {
boolean evictOnClose =
- cacheConf != null? cacheConf.shouldEvictOnClose(): true;
+ cacheConf != null? cacheConf.shouldEvictOnClose(): true;
this.closeReader(evictOnClose);
} catch (IOException ee) {
}
@@ -533,7 +529,7 @@ public class StoreFile {
*/
public void deleteReader() throws IOException {
boolean evictOnClose =
- cacheConf != null? cacheConf.shouldEvictOnClose(): true;
+ cacheConf != null? cacheConf.shouldEvictOnClose(): true;
closeReader(evictOnClose);
this.fs.delete(getPath(), true);
}
@@ -694,15 +690,11 @@ public class StoreFile {
}
public Long getMinimumTimestamp() {
- return (getReader().timeRangeTracker == null) ?
- null :
- getReader().timeRangeTracker.getMinimumTimestamp();
+ return getReader().timeRange == null? null: getReader().timeRange.getMin();
}
public Long getMaximumTimestamp() {
- return (getReader().timeRangeTracker == null) ?
- null :
- getReader().timeRangeTracker.getMaximumTimestamp();
+ return getReader().timeRange == null? null: getReader().timeRange.getMax();
}
@@ -763,13 +755,14 @@ public class StoreFile {
protected int bytesPerChecksum;
TimeRangeTracker timeRangeTracker = new TimeRangeTracker();
- /* isTimeRangeTrackerSet keeps track if the timeRange has already been set
- * When flushing a memstore, we set TimeRange and use this variable to
- * indicate that it doesn't need to be calculated again while
- * appending KeyValues.
- * It is not set in cases of compactions when it is recalculated using only
- * the appended KeyValues*/
- boolean isTimeRangeTrackerSet = false;
+ /**
+ * timeRangeTrackerSet is used to figure if we were passed a filled-out
TimeRangeTracker or not.
+ * When flushing a memstore, we set the TimeRangeTracker that it
accumulated during updates to
+ * memstore in here into this Writer and use this variable to indicate
that we do not need to
+ * recalculate the timeRangeTracker bounds; it was done already as part of
add-to-memstore.
+ * A completed TimeRangeTracker is not set in cases of compactions when it
is recalculated.
+ */
+ boolean timeRangeTrackerSet = false;
protected HFile.Writer writer;
@@ -853,12 +846,16 @@ public class StoreFile {
}
/**
- * Set TimeRangeTracker
- * @param trt
+ * Set TimeRangeTracker.
+ * Called when flushing to pass us a pre-calculated TimeRangeTracker, one
made during updates
+ * to memstore so we don't have to make one ourselves as Cells get
appended. Call before first
+ * append. If this method is not called, we will calculate our own range
of the Cells that
+ * comprise this StoreFile (and write them on the end as metadata). It is
good to have this stuff
+ * passed because it is expensive to make.
*/
public void setTimeRangeTracker(final TimeRangeTracker trt) {
this.timeRangeTracker = trt;
- isTimeRangeTrackerSet = true;
+ timeRangeTrackerSet = true;
}
/**
@@ -872,7 +869,7 @@ public class StoreFile {
if (KeyValue.Type.Put.getCode() == kv.getTypeByte()) {
earliestPutTs = Math.min(earliestPutTs, kv.getTimestamp());
}
- if (!isTimeRangeTrackerSet) {
+ if (!timeRangeTrackerSet) {
timeRangeTracker.includeTimestamp(kv);
}
}
@@ -1071,7 +1068,7 @@ public class StoreFile {
protected BloomFilter deleteFamilyBloomFilter = null;
protected BloomType bloomFilterType;
private final HFile.Reader reader;
- protected TimeRangeTracker timeRangeTracker = null;
+ protected TimeRange timeRange;
protected long sequenceID = -1;
private byte[] lastBloomKey;
private long deleteFamilyCnt = -1;
@@ -1181,13 +1178,9 @@ public class StoreFile {
* determined by the column family's TTL
* @return false if queried keys definitely don't exist in this StoreFile
*/
- boolean passesTimerangeFilter(Scan scan, long oldestUnexpiredTS) {
- if (timeRangeTracker == null) {
- return true;
- } else {
- return timeRangeTracker.includesTimeRange(scan.getTimeRange()) &&
- timeRangeTracker.getMaximumTimestamp() >= oldestUnexpiredTS;
- }
+ boolean passesTimerangeFilter(TimeRange tr, long oldestUnexpiredTS) {
+ return this.timeRange == null? true:
+ this.timeRange.includesTimeRange(tr) && this.timeRange.getMax() >=
oldestUnexpiredTS;
}
/**
@@ -1579,7 +1572,7 @@ public class StoreFile {
}
public long getMaxTimestamp() {
- return timeRangeTracker == null ? Long.MAX_VALUE :
timeRangeTracker.getMaximumTimestamp();
+ return timeRange == null ? Long.MAX_VALUE : timeRange.getMax();
}
public void setBulkLoaded(boolean bulkLoadResult) {
http://git-wip-us.apache.org/repos/asf/hbase/blob/a0bb623c/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
----------------------------------------------------------------------
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
index 4f93b00..0a0fed7 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
@@ -430,7 +430,7 @@ public class StoreFileScanner implements KeyValueScanner {
@Override
public boolean shouldUseScanner(Scan scan, SortedSet<byte[]> columns, long
oldestUnexpiredTS) {
- return reader.passesTimerangeFilter(scan, oldestUnexpiredTS)
+ return reader.passesTimerangeFilter(scan.getTimeRange(), oldestUnexpiredTS)
&& reader.passesKeyRangeFilter(scan) && reader.passesBloomFilter(scan,
columns);
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/a0bb623c/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 a93f828..2993fe4 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,72 +22,76 @@ 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.hbase.util.Writables;
import org.apache.hadoop.io.Writable;
/**
- * Stores the minimum and maximum timestamp values (both are inclusive).
- * Can be used to find if any given time range overlaps with its time range
- * MemStores use this class to track its minimum and maximum timestamps.
- * When writing StoreFiles, this information is stored in meta blocks and used
- * at read time to match against the required TimeRange.
+ * Stores minimum and maximum timestamp values.
+ * Use this class at write-time ONLY. Too much synchronization to use at read
time
+ * (TODO: there are two scenarios writing, once when lots of concurrency as
part of memstore
+ * updates but then later we can make one as part of a compaction when there
is only one thread
+ * involved -- consider making different version, the synchronized and the
unsynchronized).
+ * Use {@link TimeRange} at read time instead of this. See toTimeRange() to
make TimeRange to use.
+ * MemStores use this class to track minimum and maximum timestamps. The
TimeRangeTracker made by
+ * the MemStore is passed to the StoreFile for it to write out as part a flush
in the the file
+ * metadata. If no memstore involved -- i.e. a compaction -- then the
StoreFile will calculate its
+ * own TimeRangeTracker as it appends. The StoreFile serialized
TimeRangeTracker is used
+ * at read time via an instance of {@link TimeRange} to test if Cells fit the
StoreFile TimeRange.
*/
@InterfaceAudience.Private
public class TimeRangeTracker implements Writable {
-
- long minimumTimestamp = -1;
- long maximumTimestamp = -1;
+ static final long INITIAL_MIN_TIMESTAMP = Long.MAX_VALUE;
+ static final long INITIAL_MAX_TIMESTAMP = -1L;
+ long minimumTimestamp = INITIAL_MIN_TIMESTAMP;
+ long maximumTimestamp = INITIAL_MAX_TIMESTAMP;
/**
* 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.getMin(), trt.getMax());
}
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;
}
/**
- * Update the current TimestampRange to include the timestamp from KeyValue
- * If the Key is of type DeleteColumn or DeleteFamily, it includes the
- * entire time range from 0 to timestamp of the key.
- * @param kv the KeyValue to include
+ * @param l
+ * @return True if we initialized values
*/
- public void includeTimestamp(final KeyValue kv) {
- includeTimestamp(kv.getTimestamp());
- if (kv.isDeleteColumnOrFamily()) {
- includeTimestamp(0);
- }
+ private boolean init(final long l) {
+ if (this.minimumTimestamp != INITIAL_MIN_TIMESTAMP) return false;
+ set(l, l);
+ return true;
}
/**
- * Update the current TimestampRange to include the timestamp from Key.
+ * Update the current TimestampRange to include the timestamp from
<code>cell</code>.
* If the Key is of type DeleteColumn or DeleteFamily, it includes the
* entire time range from 0 to timestamp of the key.
- * @param key
+ * @param cell the Cell to include
*/
- 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()) {
+ public void includeTimestamp(final KeyValue kv) {
+ includeTimestamp(kv.getTimestamp());
+ if (CellUtil.isDeleteColumnOrFamily(kv)) {
includeTimestamp(0);
}
}
@@ -96,41 +100,51 @@ public class TimeRangeTracker implements Writable {
* If required, update the current TimestampRange to include timestamp
* @param timestamp the timestamp value to include
*/
- synchronized void includeTimestamp(final long timestamp) {
- if (maximumTimestamp == -1) {
- minimumTimestamp = timestamp;
- maximumTimestamp = timestamp;
- }
- else if (minimumTimestamp > timestamp) {
- minimumTimestamp = timestamp;
+ @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;
+ }
+ }
+ }
}
- else if (maximumTimestamp < timestamp) {
- maximumTimestamp = timestamp;
- }
- return;
}
/**
- * Check if the range has any overlap with TimeRange
+ * Check if the range has ANY overlap with TimeRange
* @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());
+ return (this.minimumTimestamp < tr.getMax() && this.maximumTimestamp >=
tr.getMin());
}
/**
* @return the minimumTimestamp
*/
- public synchronized long getMinimumTimestamp() {
+ public synchronized long getMin() {
return minimumTimestamp;
}
/**
* @return the maximumTimestamp
*/
- public synchronized long getMaximumTimestamp() {
+ public synchronized long getMax() {
return maximumTimestamp;
}
@@ -148,4 +162,43 @@ public class TimeRangeTracker implements Writable {
public synchronized String toString() {
return "[" + minimumTimestamp + "," + maximumTimestamp + "]";
}
-}
+
+ /**
+ * @return An instance of TimeRangeTracker filled w/ the content of
serialized
+ * TimeRangeTracker in <code>timeRangeTrackerBytes</code>.
+ * @throws IOException
+ */
+ public static TimeRangeTracker getTimeRangeTracker(final byte []
timeRangeTrackerBytes)
+ throws IOException {
+ if (timeRangeTrackerBytes == null) return null;
+ TimeRangeTracker trt = new TimeRangeTracker();
+ Writables.copyWritable(timeRangeTrackerBytes, trt);
+ return trt;
+ }
+
+ /**
+ * @return An instance of a TimeRange made from the serialized
TimeRangeTracker passed in
+ * <code>timeRangeTrackerBytes</code>.
+ * @throws IOException
+ */
+ static TimeRange getTimeRange(final byte [] timeRangeTrackerBytes) throws
IOException {
+ TimeRangeTracker trt = getTimeRangeTracker(timeRangeTrackerBytes);
+ return trt == null? null: trt.toTimeRange();
+ }
+
+ /**
+ * @return Make a TimeRange from current state of <code>this</code>.
+ */
+ TimeRange toTimeRange() throws IOException {
+ long min = getMin();
+ long max = getMax();
+ // Initial TimeRangeTracker timestamps are the opposite of what you want
for a TimeRange. Fix!
+ if (min == INITIAL_MIN_TIMESTAMP) {
+ min = TimeRange.INITIAL_MIN_TIMESTAMP;
+ }
+ if (max == INITIAL_MAX_TIMESTAMP) {
+ max = TimeRange.INITIAL_MAX_TIMESTAMP;
+ }
+ return new TimeRange(min, max);
+ }
+}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/hbase/blob/a0bb623c/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java
----------------------------------------------------------------------
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java
index 432080d..68e7288 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java
@@ -275,10 +275,9 @@ public class TestHFileOutputFormat {
// unmarshall and check values.
TimeRangeTracker timeRangeTracker = new TimeRangeTracker();
Writables.copyWritable(range, timeRangeTracker);
- LOG.info(timeRangeTracker.getMinimumTimestamp() +
- "...." + timeRangeTracker.getMaximumTimestamp());
- assertEquals(1000, timeRangeTracker.getMinimumTimestamp());
- assertEquals(2000, timeRangeTracker.getMaximumTimestamp());
+ LOG.info(timeRangeTracker.getMin() + "...." + timeRangeTracker.getMax());
+ assertEquals(1000, timeRangeTracker.getMin());
+ assertEquals(2000, timeRangeTracker.getMax());
rd.close();
} finally {
if (writer != null && context != null) writer.close(context);
http://git-wip-us.apache.org/repos/asf/hbase/blob/a0bb623c/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat2.java
----------------------------------------------------------------------
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat2.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat2.java
index 76fb516..147f60d 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat2.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat2.java
@@ -353,10 +353,10 @@ public class TestHFileOutputFormat2 {
// unmarshall and check values.
TimeRangeTracker timeRangeTracker = new TimeRangeTracker();
Writables.copyWritable(range, timeRangeTracker);
- LOG.info(timeRangeTracker.getMinimumTimestamp() +
- "...." + timeRangeTracker.getMaximumTimestamp());
- assertEquals(1000, timeRangeTracker.getMinimumTimestamp());
- assertEquals(2000, timeRangeTracker.getMaximumTimestamp());
+ LOG.info(timeRangeTracker.getMin() +
+ "...." + timeRangeTracker.getMax());
+ assertEquals(1000, timeRangeTracker.getMin());
+ assertEquals(2000, timeRangeTracker.getMax());
rd.close();
} finally {
if (writer != null && context != null) writer.close(context);
http://git-wip-us.apache.org/repos/asf/hbase/blob/a0bb623c/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MockStoreFile.java
----------------------------------------------------------------------
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 3636c48..5983aa0 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
@@ -109,31 +109,29 @@ public class MockStoreFile extends StoreFile {
}
public Long getMinimumTimestamp() {
- return (timeRangeTracker == null) ?
- null :
- timeRangeTracker.getMinimumTimestamp();
+ return (timeRangeTracker == null) ?
+ null : timeRangeTracker.getMin();
}
public Long getMaximumTimestamp() {
- return (timeRangeTracker == null) ?
- null :
- timeRangeTracker.getMaximumTimestamp();
+ return (timeRangeTracker == null) ?
+ null : timeRangeTracker.getMax();
}
@Override
- public HDFSBlocksDistribution getHDFSBlockDistribution() {
- return hdfsBlocksDistribution;
+ public long getModificationTimeStamp() {
+ return modificationTime;
}
@Override
- public long getModificationTimeStamp() {
- return modificationTime;
+ public HDFSBlocksDistribution getHDFSBlockDistribution() {
+ return hdfsBlocksDistribution;
}
-
+
@Override
public StoreFile.Reader getReader() {
final long len = this.length;
- final TimeRangeTracker timeRange = this.timeRangeTracker;
+ final TimeRangeTracker timeRangeTracker = this.timeRangeTracker;
final long entries = this.entryCount;
return new StoreFile.Reader() {
@Override
@@ -143,7 +141,7 @@ public class MockStoreFile extends StoreFile {
@Override
public long getMaxTimestamp() {
- return timeRange == null ? Long.MAX_VALUE : timeRange.maximumTimestamp;
+ return timeRange == null? Long.MAX_VALUE: timeRangeTracker.getMax();
}
@Override
http://git-wip-us.apache.org/repos/asf/hbase/blob/a0bb623c/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTimeRangeTracker.java
----------------------------------------------------------------------
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 8fca399..0bba5f4 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
@@ -32,6 +32,60 @@ import org.junit.experimental.categories.Category;
public class TestTimeRangeTracker {
@Test
+ public void testAlwaysDecrementingSetsMaximum() {
+ TimeRangeTracker trr = new TimeRangeTracker();
+ trr.includeTimestamp(3);
+ trr.includeTimestamp(2);
+ trr.includeTimestamp(1);
+ assertTrue(trr.getMin() != TimeRangeTracker.INITIAL_MIN_TIMESTAMP);
+ assertTrue(trr.getMax() != -1 /*The initial max value*/);
+ }
+
+ @Test
+ public void testSimpleInRange() {
+ TimeRangeTracker trr = new TimeRangeTracker();
+ trr.includeTimestamp(0);
+ trr.includeTimestamp(2);
+ assertTrue(trr.includesTimeRange(new TimeRange(1)));
+ }
+
+ /**
+ * Run a bunch of threads against a single TimeRangeTracker and ensure we
arrive
+ * at right range. Here we do ten threads each incrementing over 100k at an
offset
+ * of the thread index; max is 10 * 10k and min is 0.
+ * @throws InterruptedException
+ */
+ @Test
+ public void testArriveAtRightAnswer() throws InterruptedException {
+ final TimeRangeTracker trr = new TimeRangeTracker();
+ final int threadCount = 10;
+ final int calls = 1000 * 1000;
+ Thread [] threads = new Thread[threadCount];
+ for (int i = 0; i < threads.length; i++) {
+ Thread t = new Thread("" + i) {
+ @Override
+ public void run() {
+ int offset = Integer.parseInt(getName());
+ boolean even = offset % 2 == 0;
+ if (even) {
+ for (int i = (offset * calls); i < calls; i++)
trr.includeTimestamp(i);
+ } else {
+ int base = offset * calls;
+ for (int i = base + calls; i >= base; i--) trr.includeTimestamp(i);
+ }
+ }
+ };
+ t.start();
+ threads[i] = t;
+ }
+ for (int i = 0; i < threads.length; i++) {
+ threads[i].join();
+ }
+ assertTrue(trr.getMax() == calls * threadCount);
+ assertTrue(trr.getMin() == 0);
+ }
+
+ @Test
public void testRangeConstruction() throws IOException {
TimeRange defaultRange = new TimeRange();
assertEquals(0L, defaultRange.getMin());
@@ -88,7 +142,7 @@ public class TestTimeRangeTracker {
for (int i = 0; i < threads.length; i++) {
threads[i].join();
}
- System.out.println(trr.getMinimumTimestamp() + " " +
trr.getMaximumTimestamp() + " " +
+ System.out.println(trr.getMin() + " " + trr.getMax() + " " +
(System.currentTimeMillis() - start));
}
}
