Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CORE-1972 don't mark rebalance complete if some partitions are not moveable #18489

Merged

Conversation

ztlpn
Copy link
Contributor

@ztlpn ztlpn commented May 14, 2024

Previously, we finished counts rebalance if no new moves could be found, even when some partitions were not moveable. This could lead to finishing too early and not achieving balanced distribution when many partitions were not moveable (e.g. because they are disabled due to recovery mode).

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.1.x
  • v23.3.x
  • v23.2.x

Release Notes

Bug Fixes

  • Don't mark partition rebalance complete if some partitions are not moveable (e.g. due to partial recovery mode)

@ztlpn
Copy link
Contributor Author

ztlpn commented May 14, 2024

/ci-repeat

ztlpn added 4 commits May 15, 2024 13:53
Previously, we finished counts rebalance if no new moves could be
found, even when some partitions were not moveable. This could lead to
finishing too early and not achieving balanced distribution when many
partitions were not moveable (e.g. because they are disabled due to
recovery mode).
Test that rebalancing after node addition eventually achieves balanced
distribution, even though some nodes were temporarily in recovery mode.
@ztlpn ztlpn force-pushed the fix-balancer-mark-rebalance-complete branch from 83f0d44 to 588ca47 Compare May 15, 2024 11:55
@ztlpn ztlpn changed the title Fix balancer mark rebalance complete CORE-1972 don't mark rebalance complete if some partitions are not moveable May 15, 2024
@ztlpn ztlpn marked this pull request as ready for review May 15, 2024 12:02
Comment on lines 1277 to +1279
_ntp,
move.current(),
move.previous());
move.previous(),
move.current());
Copy link
Member

Choose a reason for hiding this comment

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

not confusing at all!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's an assert so I guess nobody noticed 🤷‍♂️ no idea why I wrote it like this in the first place

@piyushredpanda piyushredpanda merged commit 3ec4ac3 into redpanda-data:dev May 16, 2024
19 of 20 checks passed
@vbotbuildovich
Copy link
Collaborator

/backport v24.1.x

@vbotbuildovich
Copy link
Collaborator

/backport v23.3.x

@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v23.3.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-18489-v23.3.x-38 remotes/upstream/v23.3.x
git cherry-pick -x 39a5ec43243421f1345d7c9bab7e8ab5102fd526 46e7a589ec7810c05095d65dff049f91d24c29ac d54e6b6eb87293884e1d3ad6b8139265b8f0c561 588ca47c3d5373937b4c20c83b02b6c597fb3ebd

Workflow run logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants