HBASE-22871 Move the DirScanPool out and do not use static field #504

Apache9 · 2019-08-17T04:20:21Z

No description provided.

hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/DirScanPool.java

Reidddddd · 2019-08-17T04:55:13Z

LGTM, just one nit.

Apache-HBase · 2019-08-17T07:54:43Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
0	reexec	37	Docker mode activated.
		_ Prechecks _
+1	dupname	0	No case conflicting files found.
+1	hbaseanti	0	Patch does not have any anti-patterns.
+1	@author	0	The patch does not contain any @author tags.
+1	test4tests	0	The patch appears to include 6 new or modified test files.
		_ master Compile Tests _
+1	mvninstall	316	master passed
+1	compile	51	master passed
+1	checkstyle	71	master passed
+1	shadedjars	263	branch has no errors when building our shaded downstream artifacts.
+1	javadoc	33	master passed
0	spotbugs	250	Used deprecated FindBugs config; considering switching to SpotBugs.
+1	findbugs	248	master passed
		_ Patch Compile Tests _
+1	mvninstall	290	the patch passed
+1	compile	50	the patch passed
+1	javac	50	the patch passed
+1	checkstyle	71	hbase-server: The patch generated 0 new + 113 unchanged - 5 fixed = 113 total (was 118)
+1	whitespace	0	The patch has no whitespace issues.
+1	shadedjars	263	patch has no errors when building our shaded downstream artifacts.
+1	hadoopcheck	915	Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.
+1	javadoc	34	the patch passed
+1	findbugs	249	the patch passed
		_ Other Tests _
-1	unit	8307	hbase-server in the patch failed.
+1	asflicense	24	The patch does not generate ASF License warnings.
		11602

Reason	Tests
Failed junit tests	hadoop.hbase.replication.TestReplicationSmallTests

Subsystem	Report/Notes
Docker	Client=19.03.1 Server=19.03.1 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-504/1/artifact/out/Dockerfile
GITHUB PR	#504
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
uname	Linux 80582056ea12 4.4.0-138-generic #164-Ubuntu SMP Tue Oct 2 17:16:02 UTC 2018 x86_64 GNU/Linux
Build tool	maven
Personality	/home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-504/out/precommit/personality/provided.sh
git revision	master / `43a0ec8`
Default Java	1.8.0_181
unit	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-504/1/artifact/out/patch-unit-hbase-server.txt
Test Results	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-504/1/testReport/
Max. process+thread count	4687 (vs. ulimit of 10000)
modules	C: hbase-server U: hbase-server
Console output	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-504/1/console
versions	git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11
Powered by	Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

Apache9 · 2019-08-17T12:52:31Z

Any other concerns? @Reidddddd Thanks.

openinx · 2019-08-17T13:48:07Z

hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

+    // Create cleaner thread pool
+    cleanerPool = new DirScanPool(conf);
+    // Start log cleaner thread
+    int cleanerInterval = conf.getInt("hbase.master.cleaner.interval", 600 * 1000);


Make it a static config key & default value ?

I think this is just a format change? Can do it in another issue if need as it is not introduced by the patch here.

hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/DirScanPool.java

openinx · 2019-08-17T14:19:31Z

hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java

-  private class CleanerTask extends RecursiveTask<Boolean> {
+  private final class CleanerTask extends RecursiveTask<Boolean> {
+
+    private static final long serialVersionUID = -5444212174088754172L;


nit: do we really need this serialVersiolUID ? I don't see any serialization...

The RecursiveTask implements Serializable so we need a serialVersionUID, otherwise there will be a warning.

openinx

LGTM overall.

Apache-HBase · 2019-08-17T14:49:03Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
0	reexec	66	Docker mode activated.
		_ Prechecks _
+1	dupname	0	No case conflicting files found.
+1	hbaseanti	0	Patch does not have any anti-patterns.
+1	@author	0	The patch does not contain any @author tags.
+1	test4tests	0	The patch appears to include 6 new or modified test files.
		_ master Compile Tests _
+1	mvninstall	411	master passed
+1	compile	70	master passed
+1	checkstyle	94	master passed
+1	shadedjars	308	branch has no errors when building our shaded downstream artifacts.
+1	javadoc	42	master passed
0	spotbugs	301	Used deprecated FindBugs config; considering switching to SpotBugs.
+1	findbugs	300	master passed
		_ Patch Compile Tests _
+1	mvninstall	345	the patch passed
+1	compile	66	the patch passed
+1	javac	66	the patch passed
+1	checkstyle	100	hbase-server: The patch generated 0 new + 113 unchanged - 5 fixed = 113 total (was 118)
+1	whitespace	0	The patch has no whitespace issues.
+1	shadedjars	322	patch has no errors when building our shaded downstream artifacts.
+1	hadoopcheck	1082	Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.
+1	javadoc	38	the patch passed
+1	findbugs	258	the patch passed
		_ Other Tests _
+1	unit	16092	hbase-server in the patch passed.
+1	asflicense	26	The patch does not generate ASF License warnings.
		20051

Subsystem	Report/Notes
Docker	Client=18.09.7 Server=18.09.7 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-504/2/artifact/out/Dockerfile
GITHUB PR	#504
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
uname	Linux 18ae4df26546 4.15.0-52-generic #56-Ubuntu SMP Tue Jun 4 22:49:08 UTC 2019 x86_64 GNU/Linux
Build tool	maven
Personality	/home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-504/out/precommit/personality/provided.sh
git revision	master / `43a0ec8`
Default Java	1.8.0_181
Test Results	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-504/2/testReport/
Max. process+thread count	4675 (vs. ulimit of 10000)
modules	C: hbase-server U: hbase-server
Console output	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-504/2/console
versions	git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11
Powered by	Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

Reidddddd

QA said ok, and so do i, +1.

Signed-off-by: Zheng Hu <[email protected]> Signed-off-by: Reid Chan <[email protected]>

Apache-HBase · 2019-08-17T16:47:26Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
0	reexec	64	Docker mode activated.
		_ Prechecks _
+1	dupname	0	No case conflicting files found.
+1	hbaseanti	0	Patch does not have any anti-patterns.
+1	@author	0	The patch does not contain any @author tags.
+1	test4tests	0	The patch appears to include 6 new or modified test files.
		_ master Compile Tests _
+1	mvninstall	407	master passed
+1	compile	72	master passed
+1	checkstyle	100	master passed
+1	shadedjars	359	branch has no errors when building our shaded downstream artifacts.
+1	javadoc	46	master passed
0	spotbugs	335	Used deprecated FindBugs config; considering switching to SpotBugs.
+1	findbugs	332	master passed
		_ Patch Compile Tests _
+1	mvninstall	368	the patch passed
+1	compile	68	the patch passed
+1	javac	68	the patch passed
+1	checkstyle	89	hbase-server: The patch generated 0 new + 113 unchanged - 5 fixed = 113 total (was 118)
+1	whitespace	1	The patch has no whitespace issues.
+1	shadedjars	364	patch has no errors when building our shaded downstream artifacts.
+1	hadoopcheck	1230	Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.
+1	javadoc	44	the patch passed
+1	findbugs	348	the patch passed
		_ Other Tests _
-1	unit	15489	hbase-server in the patch failed.
+1	asflicense	38	The patch does not generate ASF License warnings.
		19946

Reason	Tests
Failed junit tests	hadoop.hbase.replication.TestReplicationDisableInactivePeer
	hadoop.hbase.replication.TestReplicationSmallTests

Subsystem	Report/Notes
Docker	Client=19.03.1 Server=19.03.1 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-504/3/artifact/out/Dockerfile
GITHUB PR	#504
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
uname	Linux 99aed17d9b1d 4.4.0-154-generic #181-Ubuntu SMP Tue Jun 25 05:29:03 UTC 2019 x86_64 GNU/Linux
Build tool	maven
Personality	/home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-504/out/precommit/personality/provided.sh
git revision	master / `7903f55`
Default Java	1.8.0_181
unit	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-504/3/artifact/out/patch-unit-hbase-server.txt
Test Results	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-504/3/testReport/
Max. process+thread count	4474 (vs. ulimit of 10000)
modules	C: hbase-server U: hbase-server
Console output	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-504/3/console
versions	git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11
Powered by	Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

Reidddddd · 2019-08-23T03:40:39Z

hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java

@@ -53,21 +53,22 @@

  @ClassRule
  public static final HBaseClassTestRule CLASS_RULE =
-      HBaseClassTestRule.forClass(TestCleanerChore.class);


Try avoiding this unnecessary change.

…che#504) Signed-off-by: Zheng Hu <[email protected]> Signed-off-by: Reid Chan <[email protected]>

…che#504) Signed-off-by: Zheng Hu <[email protected]> Signed-off-by: Reid Chan <[email protected]> (cherry picked from commit f7364a6) Change-Id: I2126efcbfd71c6a3a997d84f81469d52a70fa38d

Apache9 requested a review from openinx August 17, 2019 04:20

Reidddddd reviewed Aug 17, 2019

View reviewed changes

hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/DirScanPool.java Outdated Show resolved Hide resolved

HBASE-22871 Move the DirScanPool out and do not use static field

bbb07e4

Apache9 force-pushed the HBASE-22871 branch from 70b59b7 to bbb07e4 Compare August 17, 2019 09:04

openinx reviewed Aug 17, 2019

View reviewed changes

openinx approved these changes Aug 17, 2019

View reviewed changes

Reidddddd approved these changes Aug 17, 2019

View reviewed changes

Apache9 merged commit 8cb531f into apache:master Aug 17, 2019

asfgit pushed a commit that referenced this pull request Aug 17, 2019

HBASE-22871 Move the DirScanPool out and do not use static field (#504)

c295fd7

Signed-off-by: Zheng Hu <[email protected]> Signed-off-by: Reid Chan <[email protected]>

asfgit pushed a commit that referenced this pull request Aug 17, 2019

HBASE-22871 Move the DirScanPool out and do not use static field (#504)

f7364a6

Signed-off-by: Zheng Hu <[email protected]> Signed-off-by: Reid Chan <[email protected]>

asfgit pushed a commit that referenced this pull request Aug 17, 2019

HBASE-22871 Move the DirScanPool out and do not use static field (#504)

befbcba

Signed-off-by: Zheng Hu <[email protected]> Signed-off-by: Reid Chan <[email protected]>

Reidddddd reviewed Aug 23, 2019

View reviewed changes

infraio pushed a commit to infraio/hbase that referenced this pull request Aug 17, 2020

HBASE-22871 Move the DirScanPool out and do not use static field (apa…

37dab5b

…che#504) Signed-off-by: Zheng Hu <[email protected]> Signed-off-by: Reid Chan <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HBASE-22871 Move the DirScanPool out and do not use static field #504

HBASE-22871 Move the DirScanPool out and do not use static field #504

Apache9 commented Aug 17, 2019

Reidddddd commented Aug 17, 2019

Apache-HBase commented Aug 17, 2019

Apache9 commented Aug 17, 2019

openinx Aug 17, 2019

Apache9 Aug 17, 2019

openinx Aug 17, 2019

Apache9 Aug 17, 2019

openinx left a comment

Apache-HBase commented Aug 17, 2019

Reidddddd left a comment •

edited

Loading

Apache-HBase commented Aug 17, 2019

Reidddddd Aug 23, 2019

HBASE-22871 Move the DirScanPool out and do not use static field #504

HBASE-22871 Move the DirScanPool out and do not use static field #504

Conversation

Apache9 commented Aug 17, 2019

Reidddddd commented Aug 17, 2019

Apache-HBase commented Aug 17, 2019

Apache9 commented Aug 17, 2019

openinx Aug 17, 2019

Choose a reason for hiding this comment

Apache9 Aug 17, 2019

Choose a reason for hiding this comment

openinx Aug 17, 2019

Choose a reason for hiding this comment

Apache9 Aug 17, 2019

Choose a reason for hiding this comment

openinx left a comment

Choose a reason for hiding this comment

Apache-HBase commented Aug 17, 2019

Reidddddd left a comment • edited Loading

Choose a reason for hiding this comment

Apache-HBase commented Aug 17, 2019

Reidddddd Aug 23, 2019

Choose a reason for hiding this comment

Reidddddd left a comment •

edited

Loading