hbase git commit: HBASE-15324 Jitter may cause desiredMaxFileSize overflow in ConstantSizeRegionSplitPolicy and trigger unexpected split (Yu Li)

[es...@apache.org]

Repository: hbase
Updated Branches:
  refs/heads/branch-1.2 a3d21ecb3 -> 6b8472038


HBASE-15324 Jitter may cause desiredMaxFileSize overflow in ConstantSizeRegionSplitPolicy and trigger unexpected split (Yu Li)


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/6b847203
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/6b847203
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/6b847203

Branch: refs/heads/branch-1.2
Commit: 6b8472038dd2632d20fdf8d3216c2fe86ee68580
Parents: a3d21ec
Author: stack <st...@apache.org>
Authored: Wed Mar 30 13:31:09 2016 -0700
Committer: Esteban Gutierrez <es...@apache.org>
Committed: Tue Nov 8 15:12:38 2016 -0800

----------------------------------------------------------------------
 .../ConstantSizeRegionSplitPolicy.java          | 24 ++++++++++++++++----
 .../regionserver/TestRegionSplitPolicy.java     | 20 ++++++++++++++++
 2 files changed, 40 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/6b847203/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
index 66ef712..836cec5 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
@@ -17,13 +17,15 @@
  */
 package org.apache.hadoop.hbase.regionserver;
 
-import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import com.google.common.annotations.VisibleForTesting;
+
+import java.util.Random;
+
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseInterfaceAudience;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HTableDescriptor;
-
-import java.util.Random;
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
 
 /**
  * A {@link RegionSplitPolicy} implementation which splits a region
@@ -37,8 +39,10 @@ import java.util.Random;
 @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG)
 public class ConstantSizeRegionSplitPolicy extends RegionSplitPolicy {
   private static final Random RANDOM = new Random();
+  private static final double EPSILON = 1E-6;
 
   private long desiredMaxFileSize;
+  private double jitterRate;
 
   @Override
   protected void configureForRegion(HRegion region) {
@@ -53,7 +57,14 @@ public class ConstantSizeRegionSplitPolicy extends RegionSplitPolicy {
         HConstants.DEFAULT_MAX_FILE_SIZE);
     }
     double jitter = conf.getDouble("hbase.hregion.max.filesize.jitter", 0.25D);
-    this.desiredMaxFileSize += (long)(desiredMaxFileSize * (RANDOM.nextFloat() - 0.5D) * jitter);
+    this.jitterRate = (RANDOM.nextFloat() - 0.5D) * jitter;
+    long jitterValue = (long) (this.desiredMaxFileSize * this.jitterRate);
+    // make sure the long value won't overflow with jitter
+    if (this.jitterRate > EPSILON && jitterValue > (Long.MAX_VALUE - this.desiredMaxFileSize)) {
+      this.desiredMaxFileSize = Long.MAX_VALUE;
+    } else {
+      this.desiredMaxFileSize += jitterValue;
+    }
   }
 
   @Override
@@ -80,4 +91,9 @@ public class ConstantSizeRegionSplitPolicy extends RegionSplitPolicy {
   long getDesiredMaxFileSize() {
     return desiredMaxFileSize;
   }
+
+  @VisibleForTesting
+  public boolean positiveJitterRate() {
+    return this.jitterRate > EPSILON;
+  }
 }

http://git-wip-us.apache.org/repos/asf/hbase/blob/6b847203/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
index 1168a3e..624f27d 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
@@ -320,4 +320,24 @@ public class TestRegionSplitPolicy {
     assertEquals("ijk", Bytes.toString(policy.getSplitPoint()));
   }
 
+  @Test
+  public void testConstantSizePolicyWithJitter() throws IOException {
+    conf.set(HConstants.HBASE_REGION_SPLIT_POLICY_KEY,
+      ConstantSizeRegionSplitPolicy.class.getName());
+    htd.setMaxFileSize(Long.MAX_VALUE);
+    boolean positiveJitter = false;
+    ConstantSizeRegionSplitPolicy policy = null;
+    while (!positiveJitter) {
+      policy = (ConstantSizeRegionSplitPolicy) RegionSplitPolicy.create(mockRegion, conf);
+      positiveJitter = policy.positiveJitterRate();
+    }
+    // add a store
+    HStore mockStore = Mockito.mock(HStore.class);
+    Mockito.doReturn(2000L).when(mockStore).getSize();
+    Mockito.doReturn(true).when(mockStore).canSplit();
+    stores.add(mockStore);
+    // Jitter shouldn't cause overflow when HTableDescriptor.MAX_FILESIZE set to Long.MAX_VALUE
+    assertFalse(policy.shouldSplit());
+  }
+
 }