Details
-
New Feature
-
Status: Closed
-
Major
-
Resolution: Fixed
-
1.0.0, 2.0.0
-
None
-
Reviewed
Description
Whenever a region splits it causes lots of IO (compactions are queued for a while). Because of this it's important to make sure that well distributed tables don't have all of their regions split at exactly the same time.
This is basically the same as our compaction jitter.
Attachments
Attachments
- HBASE-13412.patch
- 2 kB
- Elliott Neil Clark
- HBASE-13412-v1.patch
- 5 kB
- Elliott Neil Clark
- HBASE-13412-v2.patch
- 4 kB
- Elliott Neil Clark
- HBASE-13412-v3.patch
- 4 kB
- Elliott Neil Clark
- hbase-13412.addendum.patch
- 0.9 kB
- Jonathan Hsieh
- HBASE-13412.addendum.0.98.patch
- 3 kB
- Andrew Kyle Purtell
- HBASE-13412.addendum.0.98-2.patch
- 3 kB
- Andrew Kyle Purtell
Activity
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12723391/HBASE-13412.patch
against master branch at commit 057499474c346b28ad5ac3ab7da420814eba547d.
ATTACHMENT ID: 12723391
+1 @author. The patch does not contain any @author tags.
-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.
+1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0)
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 protoc. The applied patch does not increase the total number of protoc compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 checkstyle. The applied patch does not increase the total number of checkstyle errors
+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 lineLengths. The patch does not introduce lines longer than 100
+1 site. The mvn site goal succeeds with this patch.
-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.regionserver.TestRegionSplitPolicy
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13583//testReport/
Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13583//artifact/patchprocess/newFindbugsWarnings.html
Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13583//artifact/patchprocess/checkstyle-aggregate.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13583//console
This message is automatically generated.
stack Not a fan of ThreadLocals. These tend to hang around. You concerned about the overhead of creating one?
eclark Is the logic correct?
+ double sizeJitter = conf.getDouble("hbase.hregion.max.filesize.jitter", 0.02D); + this.desiredMaxFileSize = (long)(desiredMaxFileSize * (RANDOM.nextFloat() - 0.5D) * sizeJitter);
You mean:
+ double sizeJitter = conf.getDouble("hbase.hregion.max.filesize.jitter", 0.02D); + this.desiredMaxFileSize += (long)(desiredMaxFileSize * (RANDOM.nextFloat() - 0.5D) * sizeJitter);
(note the +=)
stack Not a fan of ThreadLocals. These tend to hang around. You concerned about the overhead of creating one?
This suggestion is a mistake on my part. I misunderstood. Please ignore.
You are correct the logic is missing the +.... Just a typo.
Patch coming with tests.
Patch without the embarrassing typo
Also fixed the test to account for jitter. Submitting to see if any other tests rely on exact number for splits.
Hamcrest sometimes works and other times doesn't.
Seems like an issue with ordering of classpath of test dependencies. So this doesn't use hamcrest at all.
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12723503/HBASE-13412-v1.patch
against master branch at commit 8c740f43093671cfd4cc2b1052d8556e0d492c13.
ATTACHMENT ID: 12723503
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 4 new or modified tests.
+1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0)
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 protoc. The applied patch does not increase the total number of protoc compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
-1 checkstyle. The applied patch generated 1922 checkstyle errors (more than the master's current 1921 errors).
+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 lineLengths. The patch does not introduce lines longer than 100
+1 site. The mvn site goal succeeds with this patch.
-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.regionserver.TestRegionSplitPolicy
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13588//testReport/
Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13588//artifact/patchprocess/newFindbugsWarnings.html
Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13588//artifact/patchprocess/checkstyle-aggregate.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13588//console
This message is automatically generated.
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12723505/HBASE-13412-v2.patch
against master branch at commit 8c740f43093671cfd4cc2b1052d8556e0d492c13.
ATTACHMENT ID: 12723505
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 4 new or modified tests.
+1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0)
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 protoc. The applied patch does not increase the total number of protoc compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
-1 checkstyle. The applied patch generated 1922 checkstyle errors (more than the master's current 1921 errors).
+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 lineLengths. The patch does not introduce lines longer than 100
+1 site. The mvn site goal succeeds with this patch.
+1 core tests. The patch passed unit tests in .
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13589//testReport/
Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13589//artifact/patchprocess/newFindbugsWarnings.html
Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13589//artifact/patchprocess/checkstyle-aggregate.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13589//console
This message is automatically generated.
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12723532/HBASE-13412-v3.patch
against master branch at commit 8c740f43093671cfd4cc2b1052d8556e0d492c13.
ATTACHMENT ID: 12723532
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 4 new or modified tests.
+1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0)
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 protoc. The applied patch does not increase the total number of protoc compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 checkstyle. The applied patch does not increase the total number of checkstyle errors
+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 lineLengths. The patch does not introduce lines longer than 100
+1 site. The mvn site goal succeeds with this patch.
+1 core tests. The patch passed unit tests in .
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13590//testReport/
Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13590//artifact/patchprocess/newFindbugsWarnings.html
Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13590//artifact/patchprocess/checkstyle-aggregate.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13590//console
This message is automatically generated.
Having regions split at different sizes is going to confuse operators?
Since regions split on the size of a single store file it's already pretty hard to tell exactly when a region will split. So I don't think that adding a little more uncertainty around the timing of an already background process is too awful. Though it is configurable if someone would ever want to turn it off.
Thanks for review. I'll try and get a better test for this together.
FAILURE: Integrated in HBase-1.1 #374 (See https://builds.apache.org/job/HBase-1.1/374/)
HBASE-13412 Region split decisions should have jitter (eclark: rev bbdd50b9c5ad958bfb56f21ef2fdbc057de22f54)
- hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
- hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
FAILURE: Integrated in HBase-TRUNK #6355 (See https://builds.apache.org/job/HBase-TRUNK/6355/)
HBASE-13412 Region split decisions should have jitter (eclark: rev b6756b39c2ff5675f96a6e82dc4d68ec437f01c4)
- hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
- hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
hey eclark – this patch may have made trunk flakey. 3 of the last 4 last the jenkins builds [1] [2] [3] hitting TestRegionSplitPolicy.testIncreasingToupperBoundRegionSplitPolicy failures.
I've run local and it is flakey, and after reverting locally it seems to pass consistently.
[1] https://builds.apache.org/view/H-L/view/HBase/job/HBase-TRUNK/6356/testReport/
[2]https://builds.apache.org/view/H-L/view/HBase/job/HBase-TRUNK/6357/testReport/
[3]https://builds.apache.org/view/H-L/view/HBase/job/HBase-TRUNK/6358/testReport/
I think it is a test problem
Based on :
Mockito.doReturn((long)(maxSplitSize * 1.025 + 1)).when(mockStore).getSize();
double jitter = conf.getDouble("hbase.hregion.max.filesize.jitter", 0.25D); this.desiredMaxFileSize += (long)(desiredMaxFileSize * (RANDOM.nextFloat() - 0.5D) * jitter);
I think 1.025 should be 1.25.
with the addendum patch, the failing unit test passed for me 5x in a row.
FAILURE: Integrated in HBase-TRUNK #6360 (See https://builds.apache.org/job/HBase-TRUNK/6360/)
HBASE-13412 ADDENDUM Region split decisions should have jitter (jmhsieh: rev e2a90a71143f480621ccd935a0b9477d7ee4016f)
- hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
FAILURE: Integrated in HBase-1.1 #381 (See https://builds.apache.org/job/HBase-1.1/381/)
HBASE-13412 ADDENDUM Region split decisions should have jitter (jmhsieh: rev c2bfddac136008ca1be00031aaf83f6f35792476)
- hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
Both the original commit and addendum picked back to 0.98 cleanly and the new test passes repeatedly (ran it 10 times)
This change introduces a new configuration parameter and a behavioral change for ConstantSizeRegionSplitPolicy and policies that inherit from it. The change is desirable (it's the raison d'etre of this issue) but to alleviate any compatibility concerns I committed an addendum to 0.98 only that will not apply jitter unless the new configuration parameter is explicitly set.
FAILURE: Integrated in HBase-0.98 #983 (See https://builds.apache.org/job/HBase-0.98/983/)
HBASE-13412 Region split decisions should have jitter (apurtell: rev d19b5cdae378bcbaa5e7b248359d6251d115ed83)
- hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
- hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
HBASE-13412ADDENDUM Region split decisions should have jitter (apurtell: rev ee725689641d6e61146cb3be77e3f29fa5f54606) - hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
HBASE-13412ADDENDUM Region split decisions should have jitter (apurtell: rev dfc066ee83243695b27231af2a0be5cddefa4fb1) - hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
- hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #935 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/935/)
HBASE-13412 Region split decisions should have jitter (apurtell: rev d19b5cdae378bcbaa5e7b248359d6251d115ed83)
- hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
- hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
HBASE-13412ADDENDUM Region split decisions should have jitter (apurtell: rev ee725689641d6e61146cb3be77e3f29fa5f54606) - hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
HBASE-13412ADDENDUM Region split decisions should have jitter (apurtell: rev dfc066ee83243695b27231af2a0be5cddefa4fb1) - hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
- hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
Hadoop 1 doesn't have Configuration#getDouble, use floats instead to fix the Hadoop 1 build.
FAILURE: Integrated in HBase-0.98 #985 (See https://builds.apache.org/job/HBase-0.98/985/)
HBASE-13412 ADDENDUM Region split decisions should have jitter (apurtell: rev 06e0e47cab8cab09725dd850a65264f2f2317465)
- hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
- hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
SUCCESS: Integrated in HBase-0.98-on-Hadoop-1.1 #936 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/936/)
HBASE-13412 ADDENDUM Region split decisions should have jitter (apurtell: rev 06e0e47cab8cab09725dd850a65264f2f2317465)
- hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionSplitPolicy.java
- hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
Use ThreadLocalRandom rather than make your own?
Having regions split at different sizes is going to confuse operators?