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 <stack@apache.org>
apache · Apr 15, 2016 · d815211 · d815211
1 parent 6930da7
commit d815211
Show file tree

Hide file tree

Showing 17 changed files with 213 additions and 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
@@ -36,11 +36,13 @@
 @InterfaceAudience.Public
 @InterfaceStability.Stable
 public class TimeRange {
-  private static final long MIN_TIME = 0L;
+  static final long INITIAL_MIN_TIMESTAMP = 0l;
-  private static final long MAX_TIME = Long.MAX_VALUE;
+  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 TimeRange() {
    */
   public TimeRange(long minStamp) {
     this.minStamp = minStamp;
-    if (this.minStamp == MIN_TIME){
+    this.allTime = this.minStamp == MIN_TIME;
-      this.allTime = true;
-    }
   }
 
   /**
@@ -67,6 +67,7 @@ public TimeRange(long minStamp) {
    */
   public TimeRange(byte [] minStamp) {
     this.minStamp = Bytes.toLong(minStamp);
+    this.allTime = false;
   }
 
   /**
@@ -80,14 +81,12 @@ public TimeRange(long minStamp, long maxStamp) {
       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 = this.minStamp == MIN_TIME && this.maxStamp == MAX_TIME;
-      this.allTime = true;
-    }
   }
 
   /**
@@ -146,11 +145,27 @@ public boolean withinTimeRange(byte [] bytes, int offset) {
    * @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.
    * <p>

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
@@ -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,50 +49,47 @@
 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;
 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.classification.InterfaceAudience;
-import org.apache.hadoop.hbase.HRegionInfo;
+import org.apache.hadoop.hbase.classification.InterfaceStability;
-import org.apache.hadoop.hbase.KeyValueUtil;
 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 @@ private void printMeta(HFile.Reader reader, Map<byte[], byte[]> fileInfo)
         long seqid = Bytes.toLong(e.getValue());
         System.out.println(seqid);
       } else if (Bytes.compareTo(e.getKey(), Bytes.toBytes("TIMERANGE")) == 0) {
-        TimeRangeTracker timeRangeTracker = new TimeRangeTracker();
+        TimeRangeTracker timeRangeTracker = TimeRangeTracker.getTimeRangeTracker(e.getValue());
-        Writables.copyWritable(e.getValue(), timeRangeTracker);
+        System.out.println(timeRangeTracker.getMin() + "...." + timeRangeTracker.getMax());
-        System.out.println(timeRangeTracker.getMinimumTimestamp() + "...."
-            + timeRangeTracker.getMaximumTimestamp());
       } 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
@@ -161,7 +161,7 @@ protected void performMobFlush(MemStoreSnapshot snapshot, long cacheFlushId,
         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
@@ -78,8 +78,7 @@ protected AbstractMemStore(final Configuration conf, final CellComparator c) {
 
   protected void resetCellSet() {
     // Reset heap to not include any keys
-    this.active = SegmentFactory.instance().createMutableSegment(
+    this.active = SegmentFactory.instance().createMutableSegment(conf, comparator, DEEP_OVERHEAD);
-        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
@@ -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 MemStoreSnapshot snapshot(long flushOpSeqId) {
       }
     }
     return new MemStoreSnapshot(this.snapshotId, getSnapshot());
-
   }
 
   @Override
@@ -190,5 +188,4 @@ public static void main(String [] args) {
     LOG.info("Waiting " + seconds + " seconds while heap dump is taken");
     LOG.info("Exiting.");
   }
-
+}
-}
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
@@ -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 @@
  */
 @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 KeyValueScanner getKeyValueScanner() {
     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");
+  }
+}
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
@@ -26,7 +26,6 @@
  */
 @InterfaceAudience.Private
 public class MemStoreSnapshot {
-
   private final long id;
   private final int cellsCount;
   private final long size;
@@ -84,4 +83,4 @@ public KeyValueScanner getScanner() {
   public boolean isTagsPresent() {
     return this.tagsPresent;
   }
-}
+}
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
@@ -21,13 +21,13 @@
 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 long rollback(Cell cell) {
   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;
+    }
+  }
+}