HBASE-28716: Users of QuotaRetriever should pass an existing connection (branch-2) by charlesconnell · Pull Request #6054 · apache/hbase

charlesconnell · 2024-07-08T13:41:08Z

Every call to HBaseAdmin#getQuota() opens a new Connection, and then closes it. As far as I can tell, this is pointless, since it could use the existing Connection object held by the HBaseAdmin. Change this and other uses of QuotaRetriever to use existing Connections.

charlesconnell · 2024-07-08T19:19:55Z

The build check failure appears unrelated to my change

pankaj72981 · 2024-07-09T15:25:06Z

 List<QuotaSettings> quotas = new ArrayList<>();
- try (QuotaRetriever retriever = QuotaRetriever.open(conf, filter)) {
+ try (QuotaRetriever retriever = QuotaRetriever.open(connection, filter)) {
 Iterator<QuotaSettings> iterator = retriever.iterator();


Changes looks good to me.
However similar optimization can be done in master branch as well, below API is used multiple places
org.apache.hadoop.hbase.quotas.QuotaRetriever#open(org.apache.hadoop.conf.Configuration, org.apache.hadoop.hbase.quotas.QuotaFilter)
We can reuse the existing connection instead of creating new one always.

Okay, I'm happy to expand the scope of this ticket.

Apache-HBase · 2024-07-09T18:12:31Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 44s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	hbaseanti	0m 0s		Patch does not have any anti-patterns.
			_ branch-2 Compile Tests _
+0 🆗	mvndep	0m 21s		Maven dependency ordering for branch
+1 💚	mvninstall	3m 4s		branch-2 passed
+1 💚	compile	3m 41s		branch-2 passed
+1 💚	checkstyle	0m 53s		branch-2 passed
+1 💚	spotbugs	2m 21s		branch-2 passed
+1 💚	spotless	0m 46s		branch has no errors when running spotless:check.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 12s		Maven dependency ordering for patch
+1 💚	mvninstall	3m 3s		the patch passed
+1 💚	compile	3m 41s		the patch passed
+1 💚	javac	3m 41s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 52s		the patch passed
+1 💚	spotbugs	2m 42s		the patch passed
-1 ❌	hadoopcheck	11m 10s		The patch causes 10 errors with Hadoop v3.4.0.
+1 💚	spotless	0m 46s		patch has no errors when running spotless:check.
			_ Other Tests _
+1 💚	asflicense	0m 18s		The patch does not generate ASF License warnings.
		36m 33s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6054/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#6054
JIRA Issue	HBASE-28716
Optional Tests	dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
uname	Linux 320b41e37fc6 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	branch-2 / `29a462e`
Default Java	Eclipse Adoptium-11.0.23+9
hadoopcheck	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6054/2/artifact/yetus-general-check/output/patch-javac-3.4.0.txt
Max. process+thread count	79 (vs. ulimit of 30000)
modules	C: hbase-client hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6054/2/console
versions	git=2.34.1 maven=3.9.8 spotbugs=4.7.3
Powered by	Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2024-07-09T22:11:29Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 54s		Docker mode activated.
-0 ⚠️	yetus	0m 5s		Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
			_ Prechecks _
			_ branch-2 Compile Tests _
+0 🆗	mvndep	0m 12s		Maven dependency ordering for branch
+1 💚	mvninstall	2m 36s		branch-2 passed
+1 💚	compile	0m 59s		branch-2 passed
+1 💚	javadoc	0m 40s		branch-2 passed
+1 💚	shadedjars	5m 23s		branch has no errors when building our shaded downstream artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 12s		Maven dependency ordering for patch
+1 💚	mvninstall	2m 46s		the patch passed
+1 💚	compile	1m 16s		the patch passed
+1 💚	javac	1m 16s		the patch passed
+1 💚	javadoc	0m 55s		the patch passed
+1 💚	shadedjars	6m 6s		patch has no errors when building our shaded downstream artifacts.
			_ Other Tests _
+1 💚	unit	7m 41s		hbase-client in the patch passed.
+1 💚	unit	240m 24s		hbase-server in the patch passed.
		275m 30s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6054/2/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile
GITHUB PR	#6054
JIRA Issue	HBASE-28716
Optional Tests	javac javadoc unit compile shadedjars
uname	Linux 4613df4e581b 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	branch-2 / `29a462e`
Default Java	Temurin-1.8.0_412-b08
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6054/2/testReport/
Max. process+thread count	3938 (vs. ulimit of 30000)
modules	C: hbase-client hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6054/2/console
versions	git=2.34.1 maven=3.9.8
Powered by	Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2024-07-09T22:13:33Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 42s		Docker mode activated.
-0 ⚠️	yetus	0m 5s		Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
			_ Prechecks _
			_ branch-2 Compile Tests _
+0 🆗	mvndep	0m 14s		Maven dependency ordering for branch
+1 💚	mvninstall	2m 58s		branch-2 passed
+1 💚	compile	1m 19s		branch-2 passed
+1 💚	javadoc	0m 49s		branch-2 passed
+1 💚	shadedjars	5m 27s		branch has no errors when building our shaded downstream artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 11s		Maven dependency ordering for patch
+1 💚	mvninstall	3m 49s		the patch passed
+1 💚	compile	1m 42s		the patch passed
+1 💚	javac	1m 42s		the patch passed
+1 💚	javadoc	0m 57s		the patch passed
+1 💚	shadedjars	6m 42s		patch has no errors when building our shaded downstream artifacts.
			_ Other Tests _
+1 💚	unit	8m 42s		hbase-client in the patch passed.
-1 ❌	unit	239m 6s	/patch-unit-hbase-server.txt	hbase-server in the patch failed.
		277m 35s

Subsystem	Report/Notes
Docker	ClientAPI=1.46 ServerAPI=1.46 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6054/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR	#6054
JIRA Issue	HBASE-28716
Optional Tests	javac javadoc unit compile shadedjars
uname	Linux 5834d33aa3ea 5.4.0-186-generic #206-Ubuntu SMP Fri Apr 26 12:31:10 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	branch-2 / `29a462e`
Default Java	Eclipse Adoptium-17.0.11+9
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6054/2/testReport/
Max. process+thread count	4365 (vs. ulimit of 30000)
modules	C: hbase-client hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6054/2/console
versions	git=2.34.1 maven=3.9.8
Powered by	Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2024-07-09T22:16:27Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 50s		Docker mode activated.
-0 ⚠️	yetus	0m 5s		Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
			_ Prechecks _
			_ branch-2 Compile Tests _
+0 🆗	mvndep	0m 21s		Maven dependency ordering for branch
+1 💚	mvninstall	4m 24s		branch-2 passed
+1 💚	compile	1m 58s		branch-2 passed
+1 💚	javadoc	1m 0s		branch-2 passed
+1 💚	shadedjars	8m 11s		branch has no errors when building our shaded downstream artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 17s		Maven dependency ordering for patch
+1 💚	mvninstall	3m 52s		the patch passed
+1 💚	compile	1m 49s		the patch passed
+1 💚	javac	1m 49s		the patch passed
+1 💚	javadoc	0m 56s		the patch passed
+1 💚	shadedjars	5m 41s		patch has no errors when building our shaded downstream artifacts.
			_ Other Tests _
+1 💚	unit	8m 22s		hbase-client in the patch passed.
+1 💚	unit	237m 36s		hbase-server in the patch passed.
		280m 27s

Subsystem	Report/Notes
Docker	ClientAPI=1.46 ServerAPI=1.46 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6054/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR	#6054
JIRA Issue	HBASE-28716
Optional Tests	javac javadoc unit compile shadedjars
uname	Linux 1c53ce2d32fd 5.4.0-182-generic #202-Ubuntu SMP Fri Apr 26 12:29:36 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	branch-2 / `29a462e`
Default Java	Eclipse Adoptium-11.0.23+9
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6054/2/testReport/
Max. process+thread count	4536 (vs. ulimit of 30000)
modules	C: hbase-client hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6054/2/console
versions	git=2.34.1 maven=3.9.8
Powered by	Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

charlesconnell · 2024-07-15T21:44:18Z

unit test failure appears unrelated

ndimiduk · 2024-07-17T09:17:51Z

Heya -- let's keep the initial design discussion on the PR vs. master and leave this for the backport.

JIRA scope is expanded.

HBaseAdmin should pass QuotaRetriever an existing connection
d12623b

This comment has been minimized.
Sign in to view

pankaj72981 previously approved these changes Jul 9, 2024
View reviewed changes

Use existing Connection in all cases with QuotaRetriever
29a462e

charlesconnell changed the title ~~HBASE-28716: HBaseAdmin should pass QuotaRetriever an existing connection~~HBASE-28716: Users of QuotaRetriever should pass an existing connectionJul 9, 2024

charlesconnell changed the title ~~HBASE-28716: Users of QuotaRetriever should pass an existing connection~~HBASE-28716: Users of QuotaRetriever should pass an existing connection (branch-2)Jul 9, 2024

pankaj72981 self-requested a review July 9, 2024 17:26

ndimiduk added the backport This PR is a back port of some issue or issues already committed to master label Jul 17, 2024

charlesconnell closed this Jul 22, 2024

charlesconnell deleted the quota-retriver-reuse-connection-2 branch July 22, 2024 14:58

Navigation Menu

Uh oh!

HBASE-28716: Users of QuotaRetriever should pass an existing connection (branch-2) - #6054

HBASE-28716: Users of QuotaRetriever should pass an existing connection (branch-2)#6054
charlesconnell wants to merge 2 commits into
apache:branch-2from
HubSpot:quota-retriver-reuse-connection-2

charlesconnell commented Jul 8, 2024•
edited
Loading

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

charlesconnell commented Jul 8, 2024

Uh oh!

pankaj72981Jul 9, 2024

Uh oh!

charlesconnellJul 9, 2024

Uh oh!

Apache-HBase commented Jul 9, 2024

Uh oh!

Apache-HBase commented Jul 9, 2024

Uh oh!

Apache-HBase commented Jul 9, 2024

Uh oh!

Apache-HBase commented Jul 9, 2024

Uh oh!

charlesconnell commented Jul 15, 2024

Uh oh!

ndimiduk commented Jul 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

charlesconnell commented Jul 8, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

charlesconnell commented Jul 8, 2024

Uh oh!

pankaj72981Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

charlesconnellJul 9, 2024

Choose a reason for hiding this comment

Uh oh!

Apache-HBase commented Jul 9, 2024

Uh oh!

Apache-HBase commented Jul 9, 2024

Uh oh!

Apache-HBase commented Jul 9, 2024

Uh oh!

Apache-HBase commented Jul 9, 2024

Uh oh!

charlesconnell commented Jul 15, 2024

Uh oh!

ndimiduk commented Jul 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

charlesconnell commented Jul 8, 2024•
edited
Loading