From 234ddff8d45cef09b71701564295a21134e4c5ae Mon Sep 17 00:00:00 2001 From: stack Date: Wed, 13 Apr 2016 21:49:28 -0700 Subject: [PATCH] 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/ImmutableSegment.java Change ImmutableSegment so it uses a TimeRange rather than TimeRangeTracker.. it is read-only. Redo shouldSeek, getMinTimestamp, updateMetaInfo, and getTimeRangeTracker so we use TimeRange-based implementations instead of TimeRangeTracker implementations. M hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MutableSegment.java Implement shouldSeek, getMinTimestamp, updateMetaInfo, and getTimeRangeTracker using TimeRangeTracker. M hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Segment.java Make methods that were using TimeRangeTracker abstract and instead have the implementations do these methods how they want either using TimeRangeTracker when a mutable segment or TimeRange when an immutable segment. M hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Change Reader to use TimeRange-based checks instead of TimeRangeTracker. --- .../java/org/apache/hadoop/hbase/io/TimeRange.java | 25 +++++++---- .../hadoop/hbase/io/hfile/HFilePrettyPrinter.java | 27 ++++++------ .../hadoop/hbase/mob/DefaultMobStoreFlusher.java | 2 +- .../hbase/regionserver/ImmutableSegment.java | 31 +++++++++++++- .../hadoop/hbase/regionserver/MutableSegment.java | 34 ++++++++++++++- .../apache/hadoop/hbase/regionserver/Segment.java | 45 +++++++------------- .../hadoop/hbase/regionserver/StoreFile.java | 22 +++------- .../hadoop/hbase/regionserver/StoreFileReader.java | 12 ++---- .../hbase/regionserver/TimeRangeTracker.java | 48 ++++++++++++++++++---- .../hbase/regionserver/compactions/Compactor.java | 10 +---- .../hbase/mapreduce/TestHFileOutputFormat2.java | 8 ++-- .../hadoop/hbase/regionserver/MockStoreFile.java | 8 ++-- .../hbase/regionserver/TestTimeRangeTracker.java | 10 ++--- 13 files changed, 172 insertions(+), 110 deletions(-) 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 212ad45..af46f67 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 @@ -40,7 +40,7 @@ public class TimeRange { private static final long MAX_TIME = Long.MAX_VALUE; private long minStamp = MIN_TIME; private long maxStamp = MAX_TIME; - private boolean allTime = false; + private final boolean allTime; /** * Default constructor. @@ -56,9 +56,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 +65,7 @@ public class TimeRange { */ public TimeRange(byte [] minStamp) { this.minStamp = Bytes.toLong(minStamp); + this.allTime = false; } /** @@ -85,9 +84,7 @@ public class TimeRange { } 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; } /** @@ -146,12 +143,24 @@ public class TimeRange { * @return true if within TimeRange, false if not */ public boolean withinTimeRange(long timestamp) { - if(allTime) return true; + if (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 synchronized boolean includesTimeRange(final TimeRange tr) { + if (this.allTime) return true; + return getMin() < tr.getMax() && getMax() >= tr.getMin(); + } + + /** * Check if the specified timestamp is within this TimeRange. *

* Returns true if within interval [minStamp, maxStamp), false 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 cc202d4..e2a698e 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 @@ -19,6 +19,8 @@ */ package org.apache.hadoop.hbase.io.hfile; +import static com.codahale.metrics.MetricRegistry.name; + import java.io.ByteArrayOutputStream; import java.io.DataInput; import java.io.IOException; @@ -47,8 +49,6 @@ import org.apache.commons.cli.PosixParser; import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.apache.hadoop.hbase.classification.InterfaceAudience; -import org.apache.hadoop.hbase.classification.InterfaceStability; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.conf.Configured; import org.apache.hadoop.fs.FileSystem; @@ -56,41 +56,40 @@ import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellComparator; import org.apache.hadoop.hbase.CellUtil; +import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HBaseInterfaceAudience; import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.HRegionInfo; +import org.apache.hadoop.hbase.KeyValueUtil; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.Tag; import org.apache.hadoop.hbase.TagUtil; -import org.apache.hadoop.hbase.HBaseConfiguration; -import org.apache.hadoop.hbase.HRegionInfo; -import org.apache.hadoop.hbase.KeyValueUtil; +import org.apache.hadoop.hbase.classification.InterfaceAudience; +import org.apache.hadoop.hbase.classification.InterfaceStability; import org.apache.hadoop.hbase.io.FSDataInputStreamWrapper; import org.apache.hadoop.hbase.io.hfile.HFile.FileInfo; import org.apache.hadoop.hbase.mob.MobUtils; import org.apache.hadoop.hbase.regionserver.TimeRangeTracker; import org.apache.hadoop.hbase.util.BloomFilter; -import org.apache.hadoop.hbase.util.BloomFilterUtil; import org.apache.hadoop.hbase.util.BloomFilterFactory; +import org.apache.hadoop.hbase.util.BloomFilterUtil; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.FSUtils; import org.apache.hadoop.hbase.util.HFileArchiveUtil; -import org.apache.hadoop.hbase.util.Writables; import org.apache.hadoop.util.Tool; import org.apache.hadoop.util.ToolRunner; -import com.codahale.metrics.Histogram; +import com.codahale.metrics.ConsoleReporter; import com.codahale.metrics.Counter; import com.codahale.metrics.Gauge; +import com.codahale.metrics.Histogram; import com.codahale.metrics.Meter; import com.codahale.metrics.MetricFilter; import com.codahale.metrics.MetricRegistry; -import com.codahale.metrics.ConsoleReporter; import com.codahale.metrics.ScheduledReporter; import com.codahale.metrics.Snapshot; import com.codahale.metrics.Timer; -import static com.codahale.metrics.MetricRegistry.name; - /** * Implements pretty-printing functionality for {@link HFile}s. */ @@ -505,10 +504,8 @@ public class HFilePrettyPrinter extends Configured implements Tool { long seqid = Bytes.toLong(e.getValue()); System.out.println(seqid); } else if (Bytes.compareTo(e.getKey(), Bytes.toBytes("TIMERANGE")) == 0) { - TimeRangeTracker timeRangeTracker = new TimeRangeTracker(); - Writables.copyWritable(e.getValue(), timeRangeTracker); - System.out.println(timeRangeTracker.getMinimumTimestamp() + "...." - + timeRangeTracker.getMaximumTimestamp()); + TimeRangeTracker timeRangeTracker = TimeRangeTracker.getTimeRangeTracker(e.getValue()); + System.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) { System.out.println(Bytes.toInt(e.getValue())); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreFlusher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreFlusher.java index 04a8782..93fa327 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreFlusher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreFlusher.java @@ -161,7 +161,7 @@ public class DefaultMobStoreFlusher extends DefaultStoreFlusher { HConstants.COMPACTION_KV_MAX_DEFAULT); long mobCount = 0; long mobSize = 0; - long time = snapshot.getTimeRangeTracker().getMaximumTimestamp(); + long time = snapshot.getTimeRangeTracker().getMax(); mobFileWriter = mobStore.createWriterInTmp(new Date(time), snapshot.getCellsCount(), store.getFamily().getCompression(), store.getRegionInfo().getStartKey()); // the target path is {tableName}/.mob/{cfName}/mobFiles diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ImmutableSegment.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ImmutableSegment.java index 077c270..8213cea 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ImmutableSegment.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ImmutableSegment.java @@ -18,7 +18,10 @@ */ package org.apache.hadoop.hbase.regionserver; +import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.classification.InterfaceAudience; +import org.apache.hadoop.hbase.client.Scan; +import org.apache.hadoop.hbase.io.TimeRange; import org.apache.hadoop.hbase.util.CollectionBackedScanner; /** @@ -29,9 +32,15 @@ import org.apache.hadoop.hbase.util.CollectionBackedScanner; */ @InterfaceAudience.Private public class ImmutableSegment extends Segment { + /** + * This is an immutable segment so use the read-only TimeRange rather than a TimeRangeTracker. + */ + private final TimeRange timeRange; protected ImmutableSegment(Segment segment) { super(segment); + TimeRangeTracker trt = segment.getTimeRangeTracker(); + this.timeRange = trt == null? null: trt.toTimeRange(); } /** @@ -43,4 +52,24 @@ public class ImmutableSegment extends Segment { return new CollectionBackedScanner(getCellSet(), getComparator()); } -} + @Override + public boolean shouldSeek(Scan scan, long oldestUnexpiredTS) { + return this.timeRange.includesTimeRange(scan.getTimeRange()) && + this.timeRange.getMax() >= oldestUnexpiredTS; + } + + @Override + public long getMinTimestamp() { + return this.timeRange.getMin(); + } + + @Override + protected void updateMetaInfo(Cell toAdd, long s) { + throw new IllegalAccessError("This is an immutable segment"); + } + + @Override + public TimeRangeTracker getTimeRangeTracker() { + throw new IllegalAccessError("This is readonly segment; it has no TimeRangeTracker"); + } +} \ No newline at end of file diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MutableSegment.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MutableSegment.java index aef70e7..f2948ff 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MutableSegment.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MutableSegment.java @@ -21,16 +21,19 @@ package org.apache.hadoop.hbase.regionserver; import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellComparator; import org.apache.hadoop.hbase.classification.InterfaceAudience; +import org.apache.hadoop.hbase.client.Scan; /** * A mutable segment in memstore, specifically the active segment. */ @InterfaceAudience.Private public class MutableSegment extends Segment { + private final TimeRangeTracker timeRangeTracker; protected MutableSegment(CellSet cellSet, CellComparator comparator, MemStoreLAB memStoreLAB, long size) { super(cellSet, comparator, memStoreLAB, size); + this.timeRangeTracker = new TimeRangeTracker(); } /** @@ -65,4 +68,33 @@ public class MutableSegment extends Segment { Cell first() { return this.getCellSet().first(); } -} + + @Override + public boolean shouldSeek(Scan scan, long oldestUnexpiredTS) { + return (getTimeRangeTracker().includesTimeRange(scan.getTimeRange()) + && (getTimeRangeTracker().getMax() >= oldestUnexpiredTS)); + } + + @Override + public long getMinTimestamp() { + return getTimeRangeTracker().getMin(); + } + + @Override + protected void updateMetaInfo(Cell toAdd, long s) { + getTimeRangeTracker().includeTimestamp(toAdd); + size.addAndGet(s); + // In no tags case this NoTagsKeyValue.getTagsLength() is a cheap call. + // When we use ACL CP or Visibility CP which deals with Tags during + // mutation, the TagRewriteCell.getTagsLength() is a cheaper call. We do not + // parse the byte[] to identify the tags length. + if(toAdd.getTagsLength() > 0) { + tagsPresent = true; + } + } + + @Override + public TimeRangeTracker getTimeRangeTracker() { + return this.timeRangeTracker; + } +} \ No newline at end of file diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Segment.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Segment.java index 135def9..90294db 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Segment.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Segment.java @@ -40,21 +40,18 @@ import org.apache.hadoop.hbase.util.ByteRange; */ @InterfaceAudience.Private public abstract class Segment { - private volatile CellSet cellSet; private final CellComparator comparator; private volatile MemStoreLAB memStoreLAB; - private final AtomicLong size; - private final TimeRangeTracker timeRangeTracker; + protected final AtomicLong size; protected volatile boolean tagsPresent; - protected Segment(CellSet cellSet, CellComparator comparator, MemStoreLAB memStoreLAB, long - size) { + protected Segment(CellSet cellSet, CellComparator comparator, MemStoreLAB memStoreLAB, + long size) { this.cellSet = cellSet; this.comparator = comparator; this.memStoreLAB = memStoreLAB; this.size = new AtomicLong(size); - this.timeRangeTracker = new TimeRangeTracker(); this.tagsPresent = false; } @@ -63,7 +60,6 @@ public abstract class Segment { this.comparator = segment.getComparator(); this.memStoreLAB = segment.getMemStoreLAB(); this.size = new AtomicLong(segment.getSize()); - this.timeRangeTracker = segment.getTimeRangeTracker(); this.tagsPresent = segment.isTagsPresent(); } @@ -139,15 +135,9 @@ public abstract class Segment { return newKv; } - public boolean shouldSeek(Scan scan, long oldestUnexpiredTS) { - return (getTimeRangeTracker().includesTimeRange(scan.getTimeRange()) - && (getTimeRangeTracker().getMaximumTimestamp() >= - oldestUnexpiredTS)); - } + public abstract boolean shouldSeek(Scan scan, long oldestUnexpiredTS); - public long getMinTimestamp() { - return getTimeRangeTracker().getMinimumTimestamp(); - } + public abstract long getMinTimestamp(); public boolean isTagsPresent() { return tagsPresent; @@ -190,9 +180,10 @@ public abstract class Segment { size.addAndGet(delta); } - public TimeRangeTracker getTimeRangeTracker() { - return timeRangeTracker; - } + /** + * Only mutable Segments have one of these. + */ + public abstract TimeRangeTracker getTimeRangeTracker(); //*** Methods for SegmentsScanner public Cell last() { @@ -238,17 +229,10 @@ public abstract class Segment { return s; } - protected void updateMetaInfo(Cell toAdd, long s) { - getTimeRangeTracker().includeTimestamp(toAdd); - size.addAndGet(s); - // In no tags case this NoTagsKeyValue.getTagsLength() is a cheap call. - // When we use ACL CP or Visibility CP which deals with Tags during - // mutation, the TagRewriteCell.getTagsLength() is a cheaper call. We do not - // parse the byte[] to identify the tags length. - if(toAdd.getTagsLength() > 0) { - tagsPresent = true; - } - } + /** + * Only mutable Segments implement this. + */ + protected abstract void updateMetaInfo(Cell toAdd, long s); /** * Returns a subset of the segment cell set, which starts with the given cell @@ -282,5 +266,4 @@ public abstract class Segment { res += "Min ts "+getMinTimestamp()+"; "; return res; } - -} +} \ No newline at end of file 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 d66d8bd..b14c6b6 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 @@ -46,7 +46,6 @@ import org.apache.hadoop.hbase.io.hfile.CacheConfig; import org.apache.hadoop.hbase.io.hfile.HFile; import org.apache.hadoop.hbase.util.BloomFilterFactory; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.util.Writables; /** * A Store data file. Stores usually have one or more of these files. They @@ -498,15 +497,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; } // initialize so we can reuse them after reader closed. firstKey = reader.getFirstKey(); @@ -530,7 +525,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) { } @@ -576,7 +571,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); } @@ -631,15 +626,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(); } @@ -648,7 +639,6 @@ public class StoreFile { * @param comparator Comparator used to compare KVs. * @return The split point row, or null if splitting is not possible, or reader is null. */ - @SuppressWarnings("deprecation") byte[] getFileSplitPoint(CellComparator comparator) throws IOException { if (this.reader == null) { LOG.warn("Storefile " + this + " Reader is null; cannot get split point"); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileReader.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileReader.java index a2ad5a4..b8301c9 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileReader.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileReader.java @@ -60,8 +60,8 @@ public class StoreFileReader { protected BloomFilter deleteFamilyBloomFilter = null; protected BloomType bloomFilterType; private final HFile.Reader reader; - protected TimeRangeTracker timeRangeTracker = null; protected long sequenceID = -1; + protected TimeRange timeRange = null; private byte[] lastBloomKey; private long deleteFamilyCnt = -1; private boolean bulkLoadResult = false; @@ -215,12 +215,8 @@ public class StoreFileReader { * @return false if queried keys definitely don't exist in this StoreFile */ boolean passesTimerangeFilter(TimeRange timeRange, long oldestUnexpiredTS) { - if (timeRangeTracker == null) { - return true; - } else { - return timeRangeTracker.includesTimeRange(timeRange) && - timeRangeTracker.getMaximumTimestamp() >= oldestUnexpiredTS; - } + return timeRange == null? true: + timeRange.includesTimeRange(timeRange) && timeRange.getMax() >= oldestUnexpiredTS; } /** @@ -634,7 +630,7 @@ public class StoreFileReader { } public long getMaxTimestamp() { - return timeRangeTracker == null ? Long.MAX_VALUE : timeRangeTracker.getMaximumTimestamp(); + return timeRange == null ? Long.MAX_VALUE : timeRange.getMax(); } boolean isSkipResetSeqId() { 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..90a8aa1 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 @@ -26,14 +26,16 @@ 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.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. + * Stores minimum and maximum timestamp values. Both timestamps are inclusive. + * Use this class at write- time ONLY. Too much synchronization to use at read time. + * Use {@link TimeRange} at read time. See toTimeRange() to make TimeRange to use. + * MemStores use this class to track 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. + * at read time via an instance of {@link TimeRange} to test if Cells fit the StoreFile TimeRange. */ @InterfaceAudience.Private public class TimeRangeTracker implements Writable { @@ -52,7 +54,7 @@ public class TimeRangeTracker implements Writable { * @param trt source TimeRangeTracker */ public TimeRangeTracker(final TimeRangeTracker trt) { - set(trt.getMinimumTimestamp(), trt.getMaximumTimestamp()); + set(trt.getMin(), trt.getMax()); } public TimeRangeTracker(long minimumTimestamp, long maximumTimestamp) { @@ -75,7 +77,7 @@ public class TimeRangeTracker implements Writable { } /** - * Update the current TimestampRange to include the timestamp from Cell + * Update the current TimestampRange to include the timestamp from cell. * If the Key is of type DeleteColumn or DeleteFamily, it includes the * entire time range from 0 to timestamp of the key. * @param cell the Cell to include @@ -128,14 +130,14 @@ public class TimeRangeTracker implements Writable { /** * @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; } @@ -153,4 +155,34 @@ 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 timeRangeTrackerBytes. + * @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 + * timeRangeTrackerBytes. + * @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 this. + */ + TimeRange toTimeRange() { + return new TimeRange(getMin(), getMax()); + } } \ No newline at end of file diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java index 0693a2b..2dbafab 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java @@ -57,7 +57,6 @@ import org.apache.hadoop.hbase.regionserver.throttle.ThroughputController; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; -import org.apache.hadoop.hbase.util.Writables; import org.apache.hadoop.util.StringUtils.TraditionalBinaryPrefix; import com.google.common.io.Closeables; @@ -188,13 +187,8 @@ public abstract class Compactor { } } tmp = fileInfo.get(StoreFile.TIMERANGE_KEY); - TimeRangeTracker trt = new TimeRangeTracker(); - if (tmp == null) { - fd.latestPutTs = HConstants.LATEST_TIMESTAMP; - } else { - Writables.copyWritable(tmp, trt); - fd.latestPutTs = trt.getMaximumTimestamp(); - } + TimeRangeTracker trt = TimeRangeTracker.getTimeRangeTracker(tmp); + fd.latestPutTs = trt == null? HConstants.LATEST_TIMESTAMP: trt.getMax(); if (LOG.isDebugEnabled()) { LOG.debug("Compacting " + file + ", keycount=" + keyCount + 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 f611be6..57c2142 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 @@ -292,10 +292,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); 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 506f554..6f29e70 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 @@ -111,12 +111,12 @@ public class MockStoreFile extends StoreFile { public Long getMinimumTimestamp() { return (timeRangeTracker == null) ? - null : timeRangeTracker.getMinimumTimestamp(); + null : timeRangeTracker.getMin(); } public Long getMaximumTimestamp() { return (timeRangeTracker == null) ? - null : timeRangeTracker.getMaximumTimestamp(); + null : timeRangeTracker.getMax(); } @Override @@ -132,7 +132,7 @@ public class MockStoreFile extends StoreFile { @Override public StoreFileReader getReader() { final long len = this.length; - final TimeRangeTracker timeRange = this.timeRangeTracker; + final TimeRangeTracker timeRangeTracker = this.timeRangeTracker; final long entries = this.entryCount; return new StoreFileReader() { @Override @@ -142,7 +142,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 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 9d49e61..a0fad0a 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 @@ -37,8 +37,8 @@ public class TestTimeRangeTracker { trr.includeTimestamp(3); trr.includeTimestamp(2); trr.includeTimestamp(1); - assertTrue(trr.getMinimumTimestamp() != TimeRangeTracker.INITIAL_MINIMUM_TIMESTAMP); - assertTrue(trr.getMaximumTimestamp() != -1 /*The initial max value*/); + assertTrue(trr.getMin() != TimeRangeTracker.INITIAL_MINIMUM_TIMESTAMP); + assertTrue(trr.getMax() != -1 /*The initial max value*/); } @Test @@ -81,8 +81,8 @@ public class TestTimeRangeTracker { for (int i = 0; i < threads.length; i++) { threads[i].join(); } - assertTrue(trr.getMaximumTimestamp() == calls * threadCount); - assertTrue(trr.getMinimumTimestamp() == 0); + assertTrue(trr.getMax() == calls * threadCount); + assertTrue(trr.getMin() == 0); } @Test @@ -142,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)); } } \ No newline at end of file -- 2.6.1