From 3ffee86c05058fef91f9c07e308759075fca962c Mon Sep 17 00:00:00 2001 From: stack Date: Thu, 14 Apr 2016 16:53:12 -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. Signed-off-by: stack --- .../java/org/apache/hadoop/hbase/io/TimeRange.java | 37 +++++++--- .../hadoop/hbase/io/hfile/HFilePrettyPrinter.java | 27 ++++---- .../hadoop/hbase/mob/DefaultMobStoreFlusher.java | 2 +- .../hbase/regionserver/AbstractMemStore.java | 3 +- .../hadoop/hbase/regionserver/DefaultMemStore.java | 5 +- .../hbase/regionserver/ImmutableSegment.java | 27 +++++++- .../hbase/regionserver/MemStoreSnapshot.java | 3 +- .../hadoop/hbase/regionserver/MutableSegment.java | 28 +++++++- .../apache/hadoop/hbase/regionserver/Segment.java | 43 ++++-------- .../hadoop/hbase/regionserver/StoreFile.java | 22 ++---- .../hadoop/hbase/regionserver/StoreFileReader.java | 14 ++-- .../hadoop/hbase/regionserver/StoreFileWriter.java | 26 +++++--- .../hbase/regionserver/TimeRangeTracker.java | 78 ++++++++++++++++++---- .../hbase/regionserver/compactions/Compactor.java | 10 +-- .../hbase/mapreduce/TestHFileOutputFormat2.java | 8 +-- .../hadoop/hbase/regionserver/MockStoreFile.java | 8 +-- .../hbase/regionserver/TestTimeRangeTracker.java | 10 +-- 17 files changed, 215 insertions(+), 136 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..d5b2509 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; + static final long INITIAL_MIN_TIMESTAMP = 0l; + private static final long MIN_TIME = INITIAL_MIN_TIMESTAMP; + 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; } /** @@ -80,14 +81,12 @@ public class TimeRange { throw new IllegalArgumentException("Timestamp cannot be negative. minStamp:" + minStamp + ", maxStamp:" + maxStamp); } - if(maxStamp < minStamp) { + if (maxStamp < minStamp) { throw new IllegalArgumentException("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; } /** @@ -146,12 +145,28 @@ public class TimeRange { * @return true if within TimeRange, false if not */ public boolean withinTimeRange(long timestamp) { - if(allTime) return true; + 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(); + } + + /** * 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/AbstractMemStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/AbstractMemStore.java index 7f1a6bb..c3724fc 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/AbstractMemStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/AbstractMemStore.java @@ -78,8 +78,7 @@ public abstract class AbstractMemStore implements MemStore { protected void resetCellSet() { // Reset heap to not include any keys - this.active = SegmentFactory.instance().createMutableSegment( - conf, comparator, DEEP_OVERHEAD); + this.active = SegmentFactory.instance().createMutableSegment(conf, comparator, DEEP_OVERHEAD); this.timeOfOldestEdit = Long.MAX_VALUE; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java index 1bb9343..3d65bca 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java @@ -1,5 +1,4 @@ /** - * * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -98,7 +97,6 @@ public class DefaultMemStore extends AbstractMemStore { } } return new MemStoreSnapshot(this.snapshotId, getSnapshot()); - } @Override @@ -190,5 +188,4 @@ public class DefaultMemStore extends AbstractMemStore { LOG.info("Waiting " + seconds + " seconds while heap dump is taken"); LOG.info("Exiting."); } - -} +} \ No newline at end of file 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..70a608d 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,16 @@ 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 the heavy-weight + * TimeRangeTracker with all its synchronization when doing time range stuff. + */ + private final TimeRange timeRange; protected ImmutableSegment(Segment segment) { super(segment); + TimeRangeTracker trt = getTimeRangeTracker(); + this.timeRange = trt == null? null: trt.toTimeRange(); } /** @@ -43,4 +53,19 @@ 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"); + } +} \ No newline at end of file diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreSnapshot.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreSnapshot.java index 28ab693..f64979f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreSnapshot.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreSnapshot.java @@ -26,7 +26,6 @@ import org.apache.hadoop.hbase.classification.InterfaceAudience; */ @InterfaceAudience.Private public class MemStoreSnapshot { - private final long id; private final int cellsCount; private final long size; @@ -84,4 +83,4 @@ public class MemStoreSnapshot { public boolean isTagsPresent() { return this.tagsPresent; } -} +} \ 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..6337657 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,13 +21,13 @@ 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 { - protected MutableSegment(CellSet cellSet, CellComparator comparator, MemStoreLAB memStoreLAB, long size) { super(cellSet, comparator, memStoreLAB, size); @@ -65,4 +65,28 @@ 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; + } + } +} \ 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..dcad5a0 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,22 +40,21 @@ 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; + private final TimeRangeTracker timeRangeTracker; - 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; + this.timeRangeTracker = new TimeRangeTracker(); } protected Segment(Segment segment) { @@ -63,8 +62,8 @@ 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(); + this.timeRangeTracker = segment.getTimeRangeTracker(); } /** @@ -139,15 +138,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; @@ -191,7 +184,7 @@ public abstract class Segment { } public TimeRangeTracker getTimeRangeTracker() { - return timeRangeTracker; + return this.timeRangeTracker; } //*** Methods for SegmentsScanner @@ -238,17 +231,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 +268,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..4e7e829 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; @@ -214,13 +214,9 @@ public class StoreFileReader { * determined by the column family's TTL * @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; - } + boolean passesTimerangeFilter(TimeRange tr, long oldestUnexpiredTS) { + return this.timeRange == null? true: + this.timeRange.includesTimeRange(tr) && this.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/StoreFileWriter.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileWriter.java index 3a7ae5e..4a42b7f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileWriter.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileWriter.java @@ -63,13 +63,14 @@ public class StoreFileWriter implements Compactor.CellSink { private long deleteFamilyCnt = 0; 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; private KeyValue.KeyOnlyKeyValue lastBloomKeyOnlyKV = null; @@ -170,11 +171,16 @@ public class StoreFileWriter implements Compactor.CellSink { } /** - * Set TimeRangeTracker + * 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; } /** @@ -187,7 +193,7 @@ public class StoreFileWriter implements Compactor.CellSink { if (KeyValue.Type.Put.getCode() == cell.getTypeByte()) { earliestPutTs = Math.min(earliestPutTs, cell.getTimestamp()); } - if (!isTimeRangeTrackerSet) { + if (!timeRangeTrackerSet) { timeRangeTracker.includeTimestamp(cell); } } 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..77cddcd 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,20 +26,28 @@ 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. - * 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. Both timestamps are inclusive. + * 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 { - static final long INITIAL_MINIMUM_TIMESTAMP = Long.MAX_VALUE; - long minimumTimestamp = INITIAL_MINIMUM_TIMESTAMP; - long maximumTimestamp = -1; + static final long INITIAL_MIN_TIMESTAMP = Long.MAX_VALUE; + long minimumTimestamp = INITIAL_MIN_TIMESTAMP; + static final long INITIAL_MAX_TIMESTAMP = -1; + long maximumTimestamp = INITIAL_MAX_TIMESTAMP; /** * Default constructor. @@ -52,7 +60,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) { @@ -69,18 +77,20 @@ 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 (this.minimumTimestamp != INITIAL_MIN_TIMESTAMP) return false; set(l, l); return true; } /** - * 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 */ public void includeTimestamp(final Cell cell) { + // TODO: Why is this necessary? We already did this when we added the Cells to the memstore. + // Won't this run-through just do nothing except slow us down? includeTimestamp(cell.getTimestamp()); if (CellUtil.isDeleteColumnOrFamily(cell)) { includeTimestamp(0); @@ -128,14 +138,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 +163,46 @@ 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(); + } + + private boolean isFreshInstance() { + return getMin() == INITIAL_MIN_TIMESTAMP && getMax() == INITIAL_MAX_TIMESTAMP; + } + + /** + * @return Make a TimeRange from current state of this. + */ + TimeRange toTimeRange() { + long min = getMin(); + long max = getMax(); + // Check for the case where the TimeRangeTracker is fresh. In that case it has + // initial values that are antithetical to a TimeRange... Return an uninitialized TimeRange + // if passed an uninitialized TimeRangeTracker. + if (isFreshInstance()) { + return new TimeRange(); + } + return new TimeRange(min, max); + } } \ 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..3b93bb9 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_MIN_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