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 2b70644..808a9f7 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 @@ -72,8 +72,7 @@ public class TimeRange { */ @Deprecated public TimeRange(byte [] minStamp) { - this.minStamp = Bytes.toLong(minStamp); - this.allTime = false; + this(Bytes.toLong(minStamp)); } /** diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFlusher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFlusher.java index 935813c..90c16f9 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFlusher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFlusher.java @@ -68,8 +68,8 @@ public class DefaultStoreFlusher extends StoreFlusher { /* isCompaction = */ false, /* includeMVCCReadpoint = */ true, /* includesTags = */ snapshot.isTagsPresent(), - /* shouldDropBehind = */ false); - writer.setTimeRangeTracker(snapshot.getTimeRangeTracker()); + /* shouldDropBehind = */ false, + snapshot.getTimeRangeTracker()); IOException e = null; try { performFlush(scanner, writer, smallestReadPoint, throughputController); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java index db66641..aab2996 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java @@ -999,6 +999,23 @@ public class HStore implements Store { boolean isCompaction, boolean includeMVCCReadpoint, boolean includesTag, boolean shouldDropBehind) throws IOException { + return createWriterInTmp(maxKeyCount, compression, isCompaction, includeMVCCReadpoint, + includesTag, shouldDropBehind, null); + } + + /* + * @param maxKeyCount + * @param compression Compression algorithm to use + * @param isCompaction whether we are creating a new file in a compaction + * @param includesMVCCReadPoint - whether to include MVCC or not + * @param includesTag - includesTag or not + * @return Writer for a new StoreFile in the tmp dir. + */ + @Override + public StoreFile.Writer createWriterInTmp(long maxKeyCount, Compression.Algorithm compression, + boolean isCompaction, boolean includeMVCCReadpoint, boolean includesTag, + boolean shouldDropBehind, final TimeRangeTracker trt) + throws IOException { final CacheConfig writerCacheConf; if (isCompaction) { // Don't cache data on write on compactions. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java index dec27ad..e7a4de5 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java @@ -211,8 +211,24 @@ public interface Store extends HeapSize, StoreConfigInformation, PropagatingConf boolean shouldDropBehind ) throws IOException; - - + /** + * @param maxKeyCount + * @param compression Compression algorithm to use + * @param isCompaction whether we are creating a new file in a compaction + * @param includeMVCCReadpoint whether we should out the MVCC readpoint + * @param shouldDropBehind should the writer drop caches behind writes + * @param trt Ready-made timetracker to use. + * @return Writer for a new StoreFile in the tmp dir. + */ + StoreFile.Writer createWriterInTmp( + long maxKeyCount, + Compression.Algorithm compression, + boolean isCompaction, + boolean includeMVCCReadpoint, + boolean includesTags, + boolean shouldDropBehind, + final TimeRangeTracker trt + ) throws IOException; // Compaction oriented methods 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 845a8d2..42182c1 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 @@ -617,6 +617,7 @@ public class StoreFile { private Path filePath; private InetSocketAddress[] favoredNodes; private HFileContext fileContext; + private TimeRangeTracker trt; public WriterBuilder(Configuration conf, CacheConfig cacheConf, FileSystem fs) { @@ -626,6 +627,17 @@ public class StoreFile { } /** + * @param trt A premade TimeRangeTracker to use rather than build one per append (building one + * of these is expensive so good to pass one in if you have one). + * @return this (for chained invocation) + */ + public WriterBuilder withTimeRangeTracker(final TimeRangeTracker trt) { + Preconditions.checkNotNull(trt); + this.trt = trt; + return this; + } + + /** * Use either this method or {@link #withFilePath}, but not both. * @param dir Path to column family directory. The directory is created if * does not exist. The file is given a unique name within this @@ -718,7 +730,7 @@ public class StoreFile { comparator = KeyValue.COMPARATOR; } return new Writer(fs, filePath, - conf, cacheConf, comparator, bloomType, maxKeyCount, favoredNodes, fileContext); + conf, cacheConf, comparator, bloomType, maxKeyCount, favoredNodes, fileContext, trt); } } @@ -794,7 +806,7 @@ public class StoreFile { private Cell lastDeleteFamilyCell = null; private long deleteFamilyCnt = 0; - TimeRangeTracker timeRangeTracker = new TimeRangeTracker(); + final TimeRangeTracker timeRangeTracker;; /** * 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 @@ -802,7 +814,10 @@ public class StoreFile { * 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; + // TODO: Create different Writers for flush and compaction time with this value set on + // construction rather than have the Writer shared with flush and compaction and then each + // append do this expensive volatile read. + private final boolean timeRangeTrackerSet; protected HFile.Writer writer; @@ -825,6 +840,33 @@ public class StoreFile { final KVComparator comparator, BloomType bloomType, long maxKeys, InetSocketAddress[] favoredNodes, HFileContext fileContext) throws IOException { + this(fs, path, conf, cacheConf, comparator, bloomType, maxKeys, favoredNodes, fileContext, + null); + } + + /** + * Creates an HFile.Writer that also write helpful meta data. + * @param fs file system to write to + * @param path file name to create + * @param conf user configuration + * @param comparator key comparator + * @param bloomType bloom filter setting + * @param maxKeys the expected maximum number of keys to be added. Was used + * for Bloom filter size in {@link HFile} format version 1. + * @param favoredNodes + * @param fileContext - The HFile context + * @param trt Ready-made timetracker to use. + * @throws IOException problem writing to FS + */ + private Writer(FileSystem fs, Path path, + final Configuration conf, + CacheConfig cacheConf, + final KVComparator comparator, BloomType bloomType, long maxKeys, + InetSocketAddress[] favoredNodes, HFileContext fileContext, + final TimeRangeTracker trt) + throws IOException { + this.timeRangeTrackerSet = trt != null; + this.timeRangeTracker = this.timeRangeTrackerSet? trt: new TimeRangeTracker(); writer = HFile.getWriterFactory(conf, cacheConf) .withPath(fs, path) .withComparator(comparator) @@ -886,19 +928,6 @@ public class StoreFile { } /** - * 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; - timeRangeTrackerSet = true; - } - - /** * Record the earlest Put timestamp. * * If the timeRangeTracker is not set, diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFlusher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFlusher.java index 0c3432c..c367b52 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFlusher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFlusher.java @@ -115,8 +115,8 @@ public class StripeStoreFlusher extends StoreFlusher { /* isCompaction = */ false, /* includeMVCCReadpoint = */ true, /* includesTags = */ true, - /* shouldDropBehind = */ false); - writer.setTimeRangeTracker(tracker); + /* shouldDropBehind = */ false, + tracker); return writer; } }; 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 f4175cc..280bcc0 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 @@ -28,6 +28,7 @@ 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; +import org.mortbay.log.Log; /** * Stores minimum and maximum timestamp values. Both timestamps are inclusive. @@ -201,6 +202,9 @@ public class TimeRangeTracker implements Writable { if (isFreshInstance()) { return new TimeRange(); } + if (min == TimeRangeTracker.INITIAL_MIN_TIMESTAMP) { + Log.warn("BAD: MINIMUM IS " + INITIAL_MIN_TIMESTAMP + " " + this); + } return new TimeRange(min, max); } -} +} \ No newline at end of file diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTimeRangeTracker.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTimeRangeTracker.java index 46fe611..cfd80e2 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 @@ -17,16 +17,46 @@ */ package org.apache.hadoop.hbase.regionserver; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import java.io.IOException; + import org.apache.hadoop.hbase.io.TimeRange; import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.apache.hadoop.hbase.util.Writables; import org.junit.Test; import org.junit.experimental.categories.Category; @Category({SmallTests.class}) public class TestTimeRangeTracker { @Test + public void testTimeRangeInitialized() { + TimeRangeTracker src = new TimeRangeTracker(); + TimeRange tr = new TimeRange(System.currentTimeMillis()); + assertFalse(src.includesTimeRange(tr)); + } + + @Test + public void testTimeRangeTrackerNullIsSameAsTimeRangeNull() throws IOException { + TimeRangeTracker src = new TimeRangeTracker(1, 2); + byte [] bytes = Writables.getBytes(src); + TimeRange tgt = TimeRangeTracker.getTimeRange(bytes); + assertEquals(src.getMin(), tgt.getMin()); + assertEquals(src.getMax(), tgt.getMax()); + } + + @Test + public void testSerialization() throws IOException { + TimeRangeTracker src = new TimeRangeTracker(1, 2); + TimeRangeTracker tgt = new TimeRangeTracker(); + Writables.copyWritable(src, tgt); + assertEquals(src.getMin(), tgt.getMin()); + assertEquals(src.getMax(), tgt.getMax()); + } + + @Test public void testAlwaysDecrementingSetsMaximum() { TimeRangeTracker trr = new TimeRangeTracker(); trr.includeTimestamp(3);