HBASE-29202 Balancer conditionals make balancer actions more likely to be approved #6821

rmdmattingly · 2025-03-19T13:11:50Z

Because we'll accept any moves that improve conditional violations, this bug can cause us to erroneously approve balancer actions.

rmdmattingly · 2025-03-19T13:12:39Z

hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerConditionals.java


-    if (isViolatingPre && isViolatingPost) {
+    if (isViolatingPre == isViolatingPost) {
      return 0;


We also want to return 0 if we had zero violations both pre & post. Prior to this fix, we'd fall through to return -1 which erroneously considered a neutral move to be an improvement

Since this was so impactful to behavior, maybe good to capture this method in a unit test?

ndimiduk · 2025-03-19T13:15:52Z

hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerConditionals.java


-    if (isViolatingPre && isViolatingPost) {
+    if (isViolatingPre == isViolatingPost) {
      return 0;


Since this was so impactful to behavior, maybe good to capture this method in a unit test?

…o be approved

rmdmattingly · 2025-03-20T17:35:19Z

.../src/test/java/org/apache/hadoop/hbase/master/balancer/TestUnattainableBalancerCostGoal.java

+/**
+ * If your minCostNeedsBalance is set too low, then the balancer should still eventually stop making
+ * moves as further cost improvements become impossible, and balancer plan calculation becomes
+ * wasteful. This test ensures that the balancer will not get stuck in a loop of continuously moving
+ * regions.
+ */
+@Category({ MasterTests.class, MediumTests.class })
+public class TestUnattainableBalancerCostGoal {


This test fails on master atm (if balancer conditionals are enabled). It passes with this patch

rmdmattingly · 2025-03-20T17:35:56Z

...lancer/src/test/java/org/apache/hadoop/hbase/master/balancer/CandidateGeneratorTestUtil.java

-      if (balancerRuns > 1000) {
+      if (balancerRuns > 10) {


I validated locally that this change is okay, will see if the tests also pass in CI. If it's taking more than 10 balancer runs for a test, then we should probably increase that tests max run time to make it more efficient

Apache-HBase · 2025-03-20T18:14:21Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 32s		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.
			_ master Compile Tests _
+1 💚	mvninstall	3m 26s		master passed
+1 💚	compile	0m 27s		master passed
+1 💚	checkstyle	0m 10s		master passed
+1 💚	spotbugs	0m 27s		master passed
+1 💚	spotless	0m 47s		branch has no errors when running spotless:check.
			_ Patch Compile Tests _
+1 💚	mvninstall	3m 7s		the patch passed
+1 💚	compile	0m 25s		the patch passed
+1 💚	javac	0m 25s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 8s		the patch passed
+1 💚	spotbugs	0m 31s		the patch passed
+1 💚	hadoopcheck	12m 6s		Patch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚	spotless	0m 45s		patch has no errors when running spotless:check.
			_ Other Tests _
+1 💚	asflicense	0m 9s		The patch does not generate ASF License warnings.
		30m 29s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6821/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#6821
Optional Tests	dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
uname	Linux 65c9c347c535 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	master / `6226730`
Default Java	Eclipse Adoptium-17.0.11+9
Max. process+thread count	81 (vs. ulimit of 30000)
modules	C: hbase-balancer U: hbase-balancer
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6821/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 · 2025-03-20T18:20:36Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 38s		Docker mode activated.
-0 ⚠️	yetus	0m 3s		Unprocessed 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 _
+1 💚	mvninstall	4m 2s		master passed
+1 💚	compile	0m 30s		master passed
+1 💚	javadoc	0m 17s		master passed
+1 💚	shadedjars	7m 59s		branch has no errors when building our shaded downstream artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	4m 38s		the patch passed
+1 💚	compile	0m 24s		the patch passed
+1 💚	javac	0m 24s		the patch passed
+1 💚	javadoc	0m 19s		the patch passed
+1 💚	shadedjars	8m 1s		patch has no errors when building our shaded downstream artifacts.
			_ Other Tests _
+1 💚	unit	8m 42s		hbase-balancer in the patch passed.
		36m 38s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6821/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR	#6821
Optional Tests	javac javadoc unit compile shadedjars
uname	Linux 308b0b617a94 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	master / `6226730`
Default Java	Eclipse Adoptium-17.0.11+9
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6821/2/testReport/
Max. process+thread count	269 (vs. ulimit of 30000)
modules	C: hbase-balancer U: hbase-balancer
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6821/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.

ndimiduk

Nice one!

ndimiduk · 2025-03-21T09:25:18Z

...lancer/src/test/java/org/apache/hadoop/hbase/master/balancer/CandidateGeneratorTestUtil.java

  private CandidateGeneratorTestUtil() {
  }

+  enum ExhaustionType {


…o be approved (#6821) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

…o be approved (apache#6821) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

…ions more likely to be approved (apache#6821) (will be in 2.7) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

…o be approved (apache#6821) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

…o be approved (#6821) (#6836) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: Ray Mattingly <rmattingly@hubspot.com>

…o be approved (#6821) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

…o be approved (#6821) (#6837) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: Ray Mattingly <rmattingly@hubspot.com>

…ions more likely to be approved (apache#6821) (will be in 2.7) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

…o be approved (apache#6821) (apache#6837) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: Ray Mattingly <rmattingly@hubspot.com>

rmdmattingly requested a review from ndimiduk March 19, 2025 13:11

rmdmattingly commented Mar 19, 2025

View reviewed changes

ndimiduk approved these changes Mar 19, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

HBASE-29202 Balancer conditionals make balancer actions more likely t…

6226730

…o be approved

rmdmattingly force-pushed the HBASE-29202 branch from e21aaaf to 6226730 Compare March 20, 2025 17:34

rmdmattingly commented Mar 20, 2025

View reviewed changes

ndimiduk approved these changes Mar 21, 2025

View reviewed changes

rmdmattingly merged commit f1dc2c1 into apache:master Mar 21, 2025
1 check passed

rmdmattingly deleted the HBASE-29202 branch March 21, 2025 12:05

rmdmattingly added a commit that referenced this pull request Mar 21, 2025

HBASE-29202 Balancer conditionals make balancer actions more likely t…

818eb5e

…o be approved (#6821) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

rmdmattingly mentioned this pull request Mar 21, 2025

Backport "HBASE-29202 Balancer conditionals make balancer actions more likely t…" to branch-3 #6836

Merged

rmdmattingly added a commit that referenced this pull request Mar 21, 2025

HBASE-29202 Balancer conditionals make balancer actions more likely t…

da4a537

…o be approved (#6821) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

rmdmattingly mentioned this pull request Mar 21, 2025

Backport "HBASE-29202 Balancer conditionals make balancer actions more likely t…" to branch-2 #6837

Merged

rmdmattingly added a commit that referenced this pull request Mar 21, 2025

HBASE-29202 Balancer conditionals make balancer actions more likely t…

0d37302

…o be approved (#6821) (#6836) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: Ray Mattingly <rmattingly@hubspot.com>

rmdmattingly added a commit that referenced this pull request Mar 21, 2025

HBASE-29202 Balancer conditionals make balancer actions more likely t…

6f3a9b9

…o be approved (#6821) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

rmdmattingly added a commit that referenced this pull request Mar 23, 2025

HBASE-29202 Balancer conditionals make balancer actions more likely t…

b2178e3

…o be approved (#6821) (#6837) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: Ray Mattingly <rmattingly@hubspot.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HBASE-29202 Balancer conditionals make balancer actions more likely to be approved #6821

HBASE-29202 Balancer conditionals make balancer actions more likely to be approved #6821

rmdmattingly commented Mar 19, 2025

Uh oh!

rmdmattingly Mar 19, 2025

Uh oh!

ndimiduk Mar 19, 2025

Uh oh!

ndimiduk Mar 19, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

rmdmattingly Mar 20, 2025

Uh oh!

rmdmattingly Mar 20, 2025

Uh oh!

Apache-HBase commented Mar 20, 2025

Uh oh!

Apache-HBase commented Mar 20, 2025

Uh oh!

ndimiduk left a comment

Uh oh!

ndimiduk Mar 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

HBASE-29202 Balancer conditionals make balancer actions more likely to be approved #6821

HBASE-29202 Balancer conditionals make balancer actions more likely to be approved #6821

Conversation

rmdmattingly commented Mar 19, 2025

Uh oh!

rmdmattingly Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

ndimiduk Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

ndimiduk Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

rmdmattingly Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

rmdmattingly Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

Apache-HBase commented Mar 20, 2025

Uh oh!

Apache-HBase commented Mar 20, 2025

Uh oh!

ndimiduk left a comment

Choose a reason for hiding this comment

Uh oh!

ndimiduk Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants