Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HBASE-22540 [Memstore] Correct counters in MemStoreChunkPool #288

Merged
merged 2 commits into from Jun 5, 2019

Conversation

@Reidddddd
Copy link
Contributor

Reidddddd commented Jun 4, 2019

Add a new counter gotChunkCount for calculating reuse ratio.
Correct createdChunkCount initial count.
Set logStats INFO level, every 5 minutes which is not very chatty, and it is useful information.

Copy link
Contributor

apurtell left a comment

+1

@@ -77,6 +77,7 @@
private static final int statThreadPeriod = 60 * 5;
private AtomicLong createdChunkCount = new AtomicLong();
private AtomicLong reusedChunkCount = new AtomicLong();
private AtomicLong gotChunkCount = new AtomicLong();

This comment has been minimized.

Copy link
@apurtell

apurtell Jun 4, 2019

Contributor

Super minor nit.
"has" instead of "got" is more common, in our code base and elsewhere

This comment has been minimized.

Copy link
@Reidddddd

Reidddddd Jun 5, 2019

Author Contributor

It's a counter for the number of method Chunk#getChunk get called. Agree, "got" is not good. What about requestedChunkCount.

@Apache-HBase

This comment has been minimized.

Copy link

Apache-HBase commented Jun 4, 2019

💔 -1 overall

Vote Subsystem Runtime Comment
0 reexec 1225 Docker mode activated.
_ Prechecks _
0 findbugs 0 Findbugs executables are not available.
+1 hbaseanti 0 Patch does not have any anti-patterns.
+1 @author 0 The patch does not contain any @author tags.
-0 test4tests 0 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.
_ branch-1 Compile Tests _
+1 mvninstall 127 branch-1 passed
+1 compile 39 branch-1 passed with JDK v1.8.0_212
+1 compile 41 branch-1 passed with JDK v1.7.0_222
+1 checkstyle 79 branch-1 passed
+1 shadedjars 166 branch has no errors when building our shaded downstream artifacts.
+1 javadoc 30 branch-1 passed with JDK v1.8.0_212
+1 javadoc 38 branch-1 passed with JDK v1.7.0_222
_ Patch Compile Tests _
+1 mvninstall 101 the patch passed
+1 compile 40 the patch passed with JDK v1.8.0_212
+1 javac 40 the patch passed
+1 compile 41 the patch passed with JDK v1.7.0_222
+1 javac 41 the patch passed
+1 checkstyle 73 hbase-server: The patch generated 0 new + 8 unchanged - 1 fixed = 8 total (was 9)
+1 whitespace 0 The patch has no whitespace issues.
+1 shadedjars 167 patch has no errors when building our shaded downstream artifacts.
+1 hadoopcheck 222 Patch does not cause any errors with Hadoop 2.8.5 2.9.2.
+1 javadoc 29 the patch passed with JDK v1.8.0_212
+1 javadoc 37 the patch passed with JDK v1.7.0_222
_ Other Tests _
-1 unit 8687 hbase-server in the patch failed.
+1 asflicense 30 The patch does not generate ASF License warnings.
11361
Reason Tests
Failed junit tests hadoop.hbase.client.TestAdmin1
Subsystem Report/Notes
Docker Client=17.05.0-ce Server=17.05.0-ce base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-288/1/artifact/out/Dockerfile
GITHUB PR #288
Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
uname Linux 4768c83df863 4.4.0-138-generic #164-Ubuntu SMP Tue Oct 2 17:16:02 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality /testptch/patchprocess/precommit/personality/provided.sh
git revision branch-1 / e058ffe
maven version: Apache Maven 3.0.5
Default Java 1.7.0_222
Multi-JDK versions /usr/lib/jvm/java-8-openjdk-amd64:1.8.0_212 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_222
unit https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-288/1/artifact/out/patch-unit-hbase-server.txt
Test Results https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-288/1/testReport/
Max. process+thread count 4389 (vs. ulimit of 10000)
modules C: hbase-server U: hbase-server
Console output https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-288/1/console
Powered by Apache Yetus 0.9.0 http://yetus.apache.org

This message was automatically generated.

@Reidddddd

This comment has been minimized.

Copy link
Contributor Author

Reidddddd commented Jun 5, 2019

2nd commit removed the parent thread info from statistics thread which looks like thisStoreOpener-79d229f8aa7a4a99bfbff213cb37ab0f-1-MemStoreChunkPool Statistics, there is only one MemStoreChunkPool instance in RS.

And address one nit, "got" > "requested".

@Reidddddd

This comment has been minimized.

Copy link
Contributor Author

Reidddddd commented Jun 5, 2019

Thank you sir, @apurtell
It's a small improvement, I think it should not occupy too much time of you.
Please allow me S&M, free to revert or ping me if any inappropriate.

@Reidddddd Reidddddd merged commit c950de7 into apache:branch-1 Jun 5, 2019
@Reidddddd Reidddddd self-assigned this Jun 5, 2019
@Apache-HBase

This comment has been minimized.

Copy link

Apache-HBase commented Jun 5, 2019

🎊 +1 overall

Vote Subsystem Runtime Comment
0 reexec 790 Docker mode activated.
_ Prechecks _
0 findbugs 1 Findbugs executables are not available.
+1 hbaseanti 0 Patch does not have any anti-patterns.
+1 @author 0 The patch does not contain any @author tags.
-0 test4tests 0 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.
_ branch-1 Compile Tests _
+1 mvninstall 113 branch-1 passed
+1 compile 37 branch-1 passed with JDK v1.8.0_212
+1 compile 42 branch-1 passed with JDK v1.7.0_222
+1 checkstyle 80 branch-1 passed
+1 shadedjars 165 branch has no errors when building our shaded downstream artifacts.
+1 javadoc 30 branch-1 passed with JDK v1.8.0_212
+1 javadoc 38 branch-1 passed with JDK v1.7.0_222
_ Patch Compile Tests _
+1 mvninstall 98 the patch passed
+1 compile 37 the patch passed with JDK v1.8.0_212
+1 javac 37 the patch passed
+1 compile 41 the patch passed with JDK v1.7.0_222
+1 javac 41 the patch passed
+1 checkstyle 81 hbase-server: The patch generated 0 new + 8 unchanged - 1 fixed = 8 total (was 9)
+1 whitespace 0 The patch has no whitespace issues.
+1 shadedjars 168 patch has no errors when building our shaded downstream artifacts.
+1 hadoopcheck 198 Patch does not cause any errors with Hadoop 2.8.5 2.9.2.
+1 javadoc 28 the patch passed with JDK v1.8.0_212
+1 javadoc 37 the patch passed with JDK v1.7.0_222
_ Other Tests _
+1 unit 6246 hbase-server in the patch passed.
+1 asflicense 25 The patch does not generate ASF License warnings.
8434
Subsystem Report/Notes
Docker Client=17.05.0-ce Server=17.05.0-ce base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-288/2/artifact/out/Dockerfile
GITHUB PR #288
Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
uname Linux 114f8c111ed4 4.4.0-131-generic #157~14.04.1-Ubuntu SMP Fri Jul 13 08:53:17 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality /testptch/patchprocess/precommit/personality/provided.sh
git revision branch-1 / 749d58f
maven version: Apache Maven 3.0.5
Default Java 1.7.0_222
Multi-JDK versions /usr/lib/jvm/java-8-openjdk-amd64:1.8.0_212 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_222
Test Results https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-288/2/testReport/
Max. process+thread count 3784 (vs. ulimit of 10000)
modules C: hbase-server U: hbase-server
Console output https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-288/2/console
Powered by Apache Yetus 0.9.0 http://yetus.apache.org

This message was automatically generated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.