Skip to content

HBASE-29843 HBaseRPCController.setPriority(final TableName tn) is nev… - #7692

Merged
Apache9 merged 1 commit into
apache:masterfrom
Apache9:HBASE-29843
Feb 25, 2026
Merged

HBASE-29843 HBaseRPCController.setPriority(final TableName tn) is nev…#7692
Apache9 merged 1 commit into
apache:masterfrom
Apache9:HBASE-29843

Conversation

@Apache9

Copy link
Copy Markdown
Contributor

…er called on branch-3+

@Apache9
Apache9 requested a review from stotyJanuary 29, 2026 15:37
@Apache9Apache9 self-assigned this Jan 29, 2026
@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 28sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗codespell0m 0scodespell was not available.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
_ master Compile Tests _
+0 🆗mvndep0m 52sMaven dependency ordering for branch
+1 💚mvninstall3m 14smaster passed
+1 💚compile4m 13smaster passed
+1 💚checkstyle1m 18smaster passed
+1 💚spotbugs2m 20smaster passed
+1 💚spotless0m 49sbranch has no errors when running spotless:check.
_ Patch Compile Tests _
+0 🆗mvndep0m 33sMaven dependency ordering for patch
+1 💚mvninstall2m 54sthe patch passed
+1 💚compile4m 10sthe patch passed
+1 💚javac4m 10sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
+1 💚checkstyle1m 15sthe patch passed
+1 💚spotbugs2m 30sthe patch passed
+1 💚hadoopcheck11m 16sPatch does not cause any errors with Hadoop 3.3.6 3.4.1.
+1 💚spotless0m 45spatch has no errors when running spotless:check.
_ Other Tests _
+1 💚asflicense0m 20sThe patch does not generate ASF License warnings.
45m 8s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7692/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#7692
Optional Testsdupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
unameLinux 8d32ef3a1499 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 toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / f68f19b
Default JavaEclipse Adoptium-17.0.11+9
Max. process+thread count84 (vs. ulimit of 30000)
modulesC: hbase-client hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7692/1/console
versionsgit=2.34.1 maven=3.9.8 spotbugs=4.7.3
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 12sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗mvndep0m 20sMaven dependency ordering for branch
+1 💚mvninstall2m 34smaster passed
+1 💚compile1m 5smaster passed
+1 💚javadoc0m 38smaster passed
+1 💚shadedjars4m 30sbranch has no errors when building our shaded downstream artifacts.
_ Patch Compile Tests _
+0 🆗mvndep0m 12sMaven dependency ordering for patch
+1 💚mvninstall2m 21sthe patch passed
+1 💚compile1m 4sthe patch passed
+1 💚javac1m 4sthe patch passed
+1 💚javadoc0m 35sthe patch passed
+1 💚shadedjars4m 30spatch has no errors when building our shaded downstream artifacts.
_ Other Tests _
+1 💚unit1m 21shbase-client in the patch passed.
-1 ❌unit222m 52s/patch-unit-hbase-server.txthbase-server in the patch failed.
246m 39s
SubsystemReport/Notes
DockerClientAPI=1.53 ServerAPI=1.53 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7692/1/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR#7692
Optional Testsjavac javadoc unit compile shadedjars
unameLinux c520210476a7 6.14.0-1018-aws #18~24.04.1-Ubuntu SMP Mon Nov 24 19:46:27 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / f68f19b
Default JavaEclipse Adoptium-17.0.11+9
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7692/1/testReport/
Max. process+thread count5040 (vs. ulimit of 30000)
modulesC: hbase-client hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7692/1/console
versionsgit=2.34.1 maven=3.9.8
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@Apache9

Copy link
Copy Markdown
ContributorAuthor

@stoty Please see if this could solve Phoenix's problem? I introduced a new setPriority method and the default implementation in the interface is call setPriority(int) and then setPriority(TableName).

Thanks.

(int) Math.min(Integer.MAX_VALUE, TimeUnit.NANOSECONDS.toMillis(timeoutNs)));
}
controller.setPriority(priority);
controller.setPriority(priority, tableName);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this change.
I think we could make this work from the Phoenix side with adding a function to https://github.com/apache/phoenix/blob/master/phoenix-core-client/src/main/java/org/apache/hadoop/hbase/ipc/controller/MetadataRpcController.java#L70

 public void setPriority(int priority, final TableName tn) {
setPriority(tn);
}

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change in place, Phoenix does not need any change, since the default method of setPriority(int, TableName) is to call setPriority(int) and then setPriority(TableName).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we call controller.setPriority(priority, tableName); on the metadataRpcController which doesn't have a setPriority(priority, tableName) method it results calling the
DelegatingHBaseRpcController's setPriority(int priority, @Nullable TableName tableName) method

It calls the delegate.setPriority(priority, tableName) ( not setPriority(int) and then setPriority(TableName) )
Where the delegate is HBaseRpcControllerImpl

So we won't call the MetadataRpcController's setPriority(TableName)

@Apache9
Apache9force-pushed the HBASE-29843 branch 2 times, most recently from 9daf5fd to 5c261a8CompareFebruary 14, 2026 14:44
@Apache9

Copy link
Copy Markdown
ContributorAuthor

@richardantal Do you think I should also include this change on 2.x branches? Or just include this in 3.x? Which is better for Phoenix?

@richardantal

Copy link
Copy Markdown
Contributor

For Phoenix it is enough to just include this change to 3.x and master branch.

@richardantalrichardantal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from phoenix side

tn != null && tn.isSystemTable() ? HConstants.SYSTEMTABLE_QOS : HConstants.NORMAL_QOS);
}

static int calcPriority(int priority, TableName tableName) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This is never used anywhere.

@PDavidPDavid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good to me. 👍

@Apache9
Apache9 merged commit ae8aefc into apache:masterFeb 25, 2026
5 of 7 checks passed
Apache9 added a commit that referenced this pull request Feb 25, 2026
…er called on branch-3+ (#7692)
Signed-off-by: Dávid Paksy <paksyd@apache.org>
Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
Reviewed-by: Richárd Antal <richardantal@apache.org>
(cherry picked from commit ae8aefc)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@Apache9@Apache-HBase@richardantal@PDavid@wchevreuil