HBASE-26782 Minor code cleanup in and around RpcExecutor #4144

ndimiduk · 2022-03-01T17:13:25Z

While working on tracing, I see some minor cleanup that can be done in the RpcScheduler and
RpcExecutor classes. The implementations of the dispatch methods don't actually throw the
exceptions defined in their signatures, and there's some simplification to be done and checkstyle
warnings to resolve while I'm in there.

ndimiduk · 2022-03-01T17:18:08Z

Since RpcScheduler is exposed to coprocessors and Phoenix, I guess this can only go to master and branch-2.

ndimiduk · 2022-03-01T17:19:48Z

I don't think these files have been touched recently, so pining a wide reviewer audience. Sorry for the spam.

ndimiduk · 2022-03-03T11:00:23Z

FYI Phoenix maintainers: @stoty @twdsilva, maybe you have some thoughts about this change?

stoty · 2022-03-03T15:08:39Z

Thanks for the heads-up @ndimiduk .

Yes, Phoenix does use custom RpcSchedulers, and would be affected by the change.

While we can probably shim over the differences, it would be easier for us if the signature wouldn't change.

We'd prefer that this wouldn't change in branch-2.4, or even better on branch-2.

ndimiduk · 2022-03-04T09:18:28Z

@stoty

Yes, Phoenix does use custom RpcSchedulers, and would be affected by the change.

While we can probably shim over the differences, it would be easier for us if the signature wouldn't change.

We'd prefer that this wouldn't change in branch-2.4, or even better on branch-2.

Okay, let me separate the throws interface change from the rest.

Apache-HBase · 2022-03-04T10:35:24Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	3m 34s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	hbaseanti	0m 0s	Patch does not have any anti-patterns.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
		_ master Compile Tests _
+1 💚	mvninstall	3m 57s	master passed
+1 💚	compile	2m 14s	master passed
+1 💚	checkstyle	0m 37s	master passed
+1 💚	spotbugs	1m 14s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	2m 23s	the patch passed
+1 💚	compile	2m 10s	the patch passed
+1 💚	javac	2m 10s	the patch passed
+1 💚	checkstyle	0m 33s	hbase-server: The patch generated 0 new + 2 unchanged - 16 fixed = 2 total (was 18)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	hadoopcheck	11m 45s	Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚	spotbugs	1m 18s	the patch passed
		_ Other Tests _
+1 💚	asflicense	0m 10s	The patch does not generate ASF License warnings.
		35m 4s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4144/3/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#4144
Optional Tests	dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile
uname	Linux f0890da88b50 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `9c037b1`
Default Java	AdoptOpenJDK-1.8.0_282-b08
Max. process+thread count	60 (vs. ulimit of 30000)
modules	C: hbase-server U: hbase-server
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4144/3/console
versions	git=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Signed-off-by: Bharath Vissapragada <bharathv@apache.org> Signed-off-by: Duo Zhang <zhangduo@apache.org>

Apache-HBase · 2022-03-07T11:20:53Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 40s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	hbaseanti	0m 0s	Patch does not have any anti-patterns.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
		_ master Compile Tests _
+1 💚	mvninstall	2m 23s	master passed
+1 💚	compile	2m 14s	master passed
+1 💚	checkstyle	0m 34s	master passed
+1 💚	spotbugs	1m 12s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	2m 21s	the patch passed
+1 💚	compile	2m 11s	the patch passed
+1 💚	javac	2m 11s	the patch passed
+1 💚	checkstyle	0m 33s	hbase-server: The patch generated 0 new + 2 unchanged - 16 fixed = 2 total (was 18)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	hadoopcheck	11m 45s	Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚	spotbugs	1m 18s	the patch passed
		_ Other Tests _
+1 💚	asflicense	0m 7s	The patch does not generate ASF License warnings.
		30m 28s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4144/4/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#4144
Optional Tests	dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile
uname	Linux 922ad315201a 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `591f781`
Default Java	AdoptOpenJDK-1.8.0_282-b08
Max. process+thread count	60 (vs. ulimit of 30000)
modules	C: hbase-server U: hbase-server
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4144/4/console
versions	git=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2022-03-07T16:14:02Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 41s	Docker mode activated.
-0 ⚠️	yetus	0m 3s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ master Compile Tests _
+1 💚	mvninstall	2m 50s	master passed
+1 💚	compile	0m 39s	master passed
+1 💚	shadedjars	3m 35s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 22s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	2m 37s	the patch passed
+1 💚	compile	0m 39s	the patch passed
+1 💚	javac	0m 39s	the patch passed
+1 💚	shadedjars	3m 35s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 22s	the patch passed
		_ Other Tests _
+1 💚	unit	306m 37s	hbase-server in the patch passed.
		323m 39s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4144/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR	#4144
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux 217a58e52a7a 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `591f781`
Default Java	AdoptOpenJDK-11.0.10+9
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4144/4/testReport/
Max. process+thread count	3122 (vs. ulimit of 30000)
modules	C: hbase-server U: hbase-server
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4144/4/console
versions	git=2.17.1 maven=3.6.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2022-03-07T17:34:01Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	1m 49s	Docker mode activated.
-0 ⚠️	yetus	0m 3s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ master Compile Tests _
+1 💚	mvninstall	3m 20s	master passed
+1 💚	compile	0m 53s	master passed
+1 💚	shadedjars	4m 17s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 29s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	2m 58s	the patch passed
+1 💚	compile	0m 42s	the patch passed
+1 💚	javac	0m 42s	the patch passed
+1 💚	shadedjars	4m 3s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 27s	the patch passed
		_ Other Tests _
+1 💚	unit	381m 44s	hbase-server in the patch passed.
		403m 29s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4144/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR	#4144
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux cea12854da49 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `591f781`
Default Java	AdoptOpenJDK-1.8.0_282-b08
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4144/4/testReport/
Max. process+thread count	3043 (vs. ulimit of 30000)
modules	C: hbase-server U: hbase-server
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4144/4/console
versions	git=2.17.1 maven=3.6.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

ndimiduk requested review from YutSean, virajjasani, binlijin, Apache9, apurtell, bharathv, busbey, jojochuang, joshelser and z-york March 1, 2022 17:19

This comment was marked as outdated.

Sign in to view

bharathv approved these changes Mar 1, 2022

View reviewed changes

This comment was marked as outdated.

Sign in to view

ndimiduk force-pushed the 26782-cleanup-RpcScheduler-RpcExecutor branch from 6b1b4f2 to c3821cd Compare March 4, 2022 09:56

ndimiduk changed the title ~~HBASE-26782 Minor code cleanup in RpcScheduler and RpcExecutor~~ HBASE-26782 Minor code cleanup in and around RpcExecutor Mar 4, 2022

Apache9 approved these changes Mar 5, 2022

View reviewed changes

HBASE-26782 Minor code cleanup in and around RpcExecutor

629fbac

Signed-off-by: Bharath Vissapragada <bharathv@apache.org> Signed-off-by: Duo Zhang <zhangduo@apache.org>

ndimiduk force-pushed the 26782-cleanup-RpcScheduler-RpcExecutor branch from c3821cd to 629fbac Compare March 7, 2022 10:47

ndimiduk merged commit f3f2aa9 into apache:master Mar 7, 2022

ndimiduk deleted the 26782-cleanup-RpcScheduler-RpcExecutor branch March 7, 2022 10:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HBASE-26782 Minor code cleanup in and around RpcExecutor #4144

HBASE-26782 Minor code cleanup in and around RpcExecutor #4144

Uh oh!

ndimiduk commented Mar 1, 2022

Uh oh!

ndimiduk commented Mar 1, 2022

Uh oh!

ndimiduk commented Mar 1, 2022

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

ndimiduk commented Mar 3, 2022

Uh oh!

This comment was marked as outdated.

stoty commented Mar 3, 2022

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

ndimiduk commented Mar 4, 2022

Uh oh!

Apache-HBase commented Mar 4, 2022

Uh oh!

Apache-HBase commented Mar 7, 2022

Uh oh!

Apache-HBase commented Mar 7, 2022

Uh oh!

Apache-HBase commented Mar 7, 2022

Uh oh!

Uh oh!

HBASE-26782 Minor code cleanup in and around RpcExecutor #4144

HBASE-26782 Minor code cleanup in and around RpcExecutor #4144

Uh oh!

Conversation

ndimiduk commented Mar 1, 2022

Uh oh!

ndimiduk commented Mar 1, 2022

Uh oh!

ndimiduk commented Mar 1, 2022

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

ndimiduk commented Mar 3, 2022

Uh oh!

This comment was marked as outdated.

stoty commented Mar 3, 2022

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

ndimiduk commented Mar 4, 2022

Uh oh!

Apache-HBase commented Mar 4, 2022

Uh oh!

Apache-HBase commented Mar 7, 2022

Uh oh!

Apache-HBase commented Mar 7, 2022

Uh oh!

Apache-HBase commented Mar 7, 2022

Uh oh!

Uh oh!