HBASE-27758 Inconsistent synchronization in MetricsUserSourceImpl #5149

bbeaudreault · 2023-03-28T13:37:43Z

Spotbugs complains because, for example, the assignment of appendHisto is synchronized but the actual usage in updateAppend is not. The first thought might be to synchronize all of the methods, but that would be detrimental to performance and unnecessary since these metrics are already thread safe.

The synchronization of register/deregister was added as part of the LossyCounting stuff for user metrics. So its possible under extreme conditions for the LossyCounting thread to deregister a user source which is currently being registered. So it's important to synchronize the actual metric creation/removal, more than anything else.

There is only one caller of register/deregister, here in MetricsUserAggregateSourceImpl. Note that it's already synchronized. So I think the synchronization here is redundant. Further, I don't think the synchronization here is even effective since it's synchronizing access to registry which could be used in other instances of MetricsUserSourceImpl.

Apache-HBase · 2023-03-28T16:50:13Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 27s	Docker mode activated.
-0 ⚠️	yetus	0m 2s	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 21s	master passed
+1 💚	compile	0m 14s	master passed
+1 💚	shadedjars	4m 47s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 12s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	2m 59s	the patch passed
+1 💚	compile	0m 12s	the patch passed
+1 💚	javac	0m 12s	the patch passed
+1 💚	shadedjars	4m 46s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 11s	the patch passed
		_ Other Tests _
+1 💚	unit	0m 32s	hbase-hadoop-compat in the patch passed.
		18m 50s

Subsystem	Report/Notes
Docker	ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR	#5149
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux b4bf92cb6386 5.4.0-1097-aws #105~18.04.1-Ubuntu SMP Mon Feb 13 17:50:57 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `efb360d`
Default Java	Temurin-1.8.0_352-b08
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/1/testReport/
Max. process+thread count	258 (vs. ulimit of 30000)
modules	C: hbase-hadoop-compat U: hbase-hadoop-compat
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/1/console
versions	git=2.34.1 maven=3.8.6
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2023-03-28T16:50:35Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 59s	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 35s	master passed
+1 💚	compile	0m 14s	master passed
+1 💚	shadedjars	4m 22s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 15s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	3m 19s	the patch passed
+1 💚	compile	0m 14s	the patch passed
+1 💚	javac	0m 14s	the patch passed
+1 💚	shadedjars	4m 21s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 12s	the patch passed
		_ Other Tests _
+1 💚	unit	0m 38s	hbase-hadoop-compat in the patch passed.
		19m 20s

Subsystem	Report/Notes
Docker	ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR	#5149
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux 077a447f9228 5.4.0-137-generic #154-Ubuntu SMP Thu Jan 5 17:03:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `efb360d`
Default Java	Eclipse Adoptium-11.0.17+8
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/1/testReport/
Max. process+thread count	277 (vs. ulimit of 30000)
modules	C: hbase-hadoop-compat U: hbase-hadoop-compat
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/1/console
versions	git=2.34.1 maven=3.8.6
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2023-03-28T17:01:54Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 24s	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 44s	master passed
+1 💚	compile	0m 18s	master passed
+1 💚	checkstyle	0m 8s	master passed
+1 💚	spotless	0m 40s	branch has no errors when running spotless:check.
-1 ❌	spotbugs	0m 22s	hbase-hadoop-compat in master has 4 extant spotbugs warnings.
		_ Patch Compile Tests _
+1 💚	mvninstall	3m 12s	the patch passed
+1 💚	compile	0m 15s	the patch passed
+1 💚	javac	0m 15s	the patch passed
+1 💚	checkstyle	0m 7s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	hadoopcheck	12m 42s	Patch does not cause any errors with Hadoop 3.2.4 3.3.4.
-1 ❌	spotless	0m 37s	patch has 1 errors when running spotless:check, run spotless:apply to fix.
+1 💚	spotbugs	0m 26s	hbase-hadoop-compat generated 0 new + 0 unchanged - 4 fixed = 0 total (was 4)
		_ Other Tests _
+1 💚	asflicense	0m 9s	The patch does not generate ASF License warnings.
		30m 36s

Subsystem	Report/Notes
Docker	ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#5149
Optional Tests	dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
uname	Linux 6175e065ccc6 5.4.0-1094-aws #102~18.04.1-Ubuntu SMP Tue Jan 10 21:07:03 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `efb360d`
Default Java	Eclipse Adoptium-11.0.17+8
spotbugs	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/1/artifact/yetus-general-check/output/branch-spotbugs-hbase-hadoop-compat-warnings.html
spotless	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/1/artifact/yetus-general-check/output/patch-spotless.txt
Max. process+thread count	85 (vs. ulimit of 30000)
modules	C: hbase-hadoop-compat U: hbase-hadoop-compat
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/1/console
versions	git=2.34.1 maven=3.8.6 spotbugs=4.7.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2023-03-30T22:04:42Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 26s	Docker mode activated.
-0 ⚠️	yetus	0m 2s	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 38s	master passed
+1 💚	compile	0m 12s	master passed
+1 💚	shadedjars	4m 46s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 11s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	3m 0s	the patch passed
+1 💚	compile	0m 12s	the patch passed
+1 💚	javac	0m 12s	the patch passed
+1 💚	shadedjars	4m 44s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 10s	the patch passed
		_ Other Tests _
+1 💚	unit	0m 32s	hbase-hadoop-compat in the patch passed.
		19m 0s

Subsystem	Report/Notes
Docker	ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR	#5149
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux 18814d1b9a79 5.4.0-1097-aws #105~18.04.1-Ubuntu SMP Mon Feb 13 17:50:57 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `e5620e2`
Default Java	Temurin-1.8.0_352-b08
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/2/testReport/
Max. process+thread count	259 (vs. ulimit of 30000)
modules	C: hbase-hadoop-compat U: hbase-hadoop-compat
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/2/console
versions	git=2.34.1 maven=3.8.6
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2023-03-30T22:04:44Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 52s	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 25s	master passed
+1 💚	compile	0m 14s	master passed
+1 💚	shadedjars	4m 21s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 13s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	3m 23s	the patch passed
+1 💚	compile	0m 14s	the patch passed
+1 💚	javac	0m 14s	the patch passed
+1 💚	shadedjars	4m 27s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 13s	the patch passed
		_ Other Tests _
+1 💚	unit	0m 35s	hbase-hadoop-compat in the patch passed.
		19m 3s

Subsystem	Report/Notes
Docker	ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR	#5149
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux 791f50683116 5.4.0-137-generic #154-Ubuntu SMP Thu Jan 5 17:03:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `e5620e2`
Default Java	Eclipse Adoptium-11.0.17+8
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/2/testReport/
Max. process+thread count	252 (vs. ulimit of 30000)
modules	C: hbase-hadoop-compat U: hbase-hadoop-compat
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/2/console
versions	git=2.34.1 maven=3.8.6
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2023-03-30T22:21:16Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 29s	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	4m 9s	master passed
+1 💚	compile	0m 19s	master passed
+1 💚	checkstyle	0m 9s	master passed
+1 💚	spotless	0m 48s	branch has no errors when running spotless:check.
-1 ❌	spotbugs	0m 22s	hbase-hadoop-compat in master has 4 extant spotbugs warnings.
		_ Patch Compile Tests _
+1 💚	mvninstall	4m 2s	the patch passed
+1 💚	compile	0m 19s	the patch passed
+1 💚	javac	0m 19s	the patch passed
+1 💚	checkstyle	0m 9s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	hadoopcheck	14m 44s	Patch does not cause any errors with Hadoop 3.2.4 3.3.4.
+1 💚	spotless	0m 46s	patch has no errors when running spotless:check.
+1 💚	spotbugs	0m 32s	hbase-hadoop-compat generated 0 new + 0 unchanged - 4 fixed = 0 total (was 4)
		_ Other Tests _
+1 💚	asflicense	0m 8s	The patch does not generate ASF License warnings.
		35m 31s

Subsystem	Report/Notes
Docker	ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#5149
Optional Tests	dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
uname	Linux de084c86d17e 5.4.0-1094-aws #102~18.04.1-Ubuntu SMP Tue Jan 10 21:07:03 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `e5620e2`
Default Java	Eclipse Adoptium-11.0.17+8
spotbugs	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/2/artifact/yetus-general-check/output/branch-spotbugs-hbase-hadoop-compat-warnings.html
Max. process+thread count	86 (vs. ulimit of 30000)
modules	C: hbase-hadoop-compat U: hbase-hadoop-compat
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5149/2/console
versions	git=2.34.1 maven=3.8.6 spotbugs=4.7.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

) Signed-off-by: Duo Zhang <zhangduo@apache.org>

…ache#5149) Signed-off-by: Duo Zhang <zhangduo@apache.org> (cherry picked from commit dedf1f9) Change-Id: I22c87caa97668aff7b7789aebb47e7dce9469789

HBASE-27758 Inconsistent synchronization in MetricsUserSourceImpl

78d810f

bbeaudreault force-pushed the HBASE-27758 branch from c90f595 to 78d810f Compare March 30, 2023 20:05

Apache9 approved these changes Mar 31, 2023

View reviewed changes

bbeaudreault merged commit 143e9b4 into apache:master Mar 31, 2023

bbeaudreault deleted the HBASE-27758 branch March 31, 2023 12:11

bbeaudreault added a commit that referenced this pull request Mar 31, 2023

HBASE-27758 Inconsistent synchronization in MetricsUserSourceImpl (#5149

cc00b8b

) Signed-off-by: Duo Zhang <zhangduo@apache.org>

bbeaudreault added a commit that referenced this pull request Mar 31, 2023

HBASE-27758 Inconsistent synchronization in MetricsUserSourceImpl (#5149

694752b

) Signed-off-by: Duo Zhang <zhangduo@apache.org>

bbeaudreault added a commit that referenced this pull request Apr 3, 2023

HBASE-27758 Inconsistent synchronization in MetricsUserSourceImpl (#5149

dedf1f9

) Signed-off-by: Duo Zhang <zhangduo@apache.org>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HBASE-27758 Inconsistent synchronization in MetricsUserSourceImpl #5149

HBASE-27758 Inconsistent synchronization in MetricsUserSourceImpl #5149

bbeaudreault commented Mar 28, 2023 •

edited

Loading

Apache-HBase commented Mar 28, 2023

Apache-HBase commented Mar 28, 2023

Apache-HBase commented Mar 28, 2023

Apache-HBase commented Mar 30, 2023

Apache-HBase commented Mar 30, 2023

Apache-HBase commented Mar 30, 2023

HBASE-27758 Inconsistent synchronization in MetricsUserSourceImpl #5149

HBASE-27758 Inconsistent synchronization in MetricsUserSourceImpl #5149

Conversation

bbeaudreault commented Mar 28, 2023 • edited Loading

Apache-HBase commented Mar 28, 2023

Apache-HBase commented Mar 28, 2023

Apache-HBase commented Mar 28, 2023

Apache-HBase commented Mar 30, 2023

Apache-HBase commented Mar 30, 2023

Apache-HBase commented Mar 30, 2023

bbeaudreault commented Mar 28, 2023 •

edited

Loading