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

Schedule counts balancing-related partition movements in partition balancer #11366

Merged
merged 14 commits into from
Jun 16, 2023

Conversation

ztlpn
Copy link
Contributor

@ztlpn ztlpn commented Jun 12, 2023

Move counts balancing code from members_backend to the partition balancer.

The algorithm is as follows:

  • Maintain a list of nodes for which reallocations_finished command hasn't been issued yet. If this list is not empty, run the balancing operator in the partition balancer planner
  • Planner simply goes over all replicas and tries to move them to a more optimal place.
  • If we haven't added any actions - this means we are at a (local) minimum and rebalancing is finished => we can issue reallocations_finished commands.

On-demand rebalancing is moved to the balancer as well.

As during counts-based rebalancing sometimes moves are generated that do not result in improvement, revert API is implemented to discard them.

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
  • v23.1.x
  • v22.3.x
  • v22.2.x

Release Notes

Improvements

  • Partition movements related to rebalancing on node addition now take free disk space and health of target nodes into account.

@ztlpn ztlpn requested review from bharathv and mmaslankaprv June 12, 2023 18:41
@ztlpn ztlpn force-pushed the pb-counts-rebalancing branch 3 times, most recently from afa0f1d to 79f3508 Compare June 13, 2023 17:32
@mmaslankaprv
Copy link
Member

this looks really good, i am wondering if we can remove update finishing logic from the members backend now ?

@ztlpn
Copy link
Contributor Author

ztlpn commented Jun 14, 2023

i am wondering if we can remove update finishing logic from the members backend now ?

We need to get rid of the recommission-related cancellations logic first.

mmaslankaprv
mmaslankaprv previously approved these changes Jun 14, 2023
@ztlpn ztlpn dismissed mmaslankaprv’s stale review June 14, 2023 12:58

The merge-base changed after approval.

@ztlpn ztlpn force-pushed the pb-counts-rebalancing branch 2 times, most recently from 3fba0c5 to d6ea177 Compare June 14, 2023 22:39
ztlpn added 8 commits June 15, 2023 01:40
We can rely on partition allocator state to detect the case when all
necessary node drain actions already have been done or scheduled.
The algorithm is simple: just try to move every replica. Assuming that
due to the min-count soft constraint it will end on a node with fewer
replicas than the original one, we will move towards (local) optimum.

This operator is triggered only if we have non-empty
"nodes-to-rebalance" collection in partition_balancer_state. It will be
updated by members_manager and will contain all nodes that have been
added but haven't yet been issued a corresponoding finish_reallocations
command.
We need to try moving partition replicas in random order to avoid
creating topic hotspots after counts rebalancing (i.e. when some of the
topic partitions are not present on added nodes).
@ztlpn ztlpn force-pushed the pb-counts-rebalancing branch from d6ea177 to c45d4f0 Compare June 14, 2023 22:40
@ztlpn
Copy link
Contributor Author

ztlpn commented Jun 14, 2023

Rebased on dev and added another change: tightened the way we handle per-term state in the backend. This way it is easier to remember to reset it when the term changes.

@ztlpn ztlpn requested a review from mmaslankaprv June 14, 2023 23:02
@ztlpn ztlpn force-pushed the pb-counts-rebalancing branch from c45d4f0 to 17045c1 Compare June 15, 2023 11:21
@ztlpn
Copy link
Contributor Author

ztlpn commented Jun 15, 2023

Change in force-push: added controller term check when issuing finish_reallocations commands.

ztlpn added 5 commits June 15, 2023 14:38
Term check can help to prevent possible inconsistencies from several
rebalancing processes running in different controller terms.
Additionally, remove the ability to call this on a non-controller leader
node, because this method is not user-facing and only used by
rebalancing backends.
Some of the state in the balancer backend becomes obsolete when the term
changes. To make it easy to reset all this state in one go, gather it in
a single struct.
Counts rebalancing is now handled by partition balancer.
As members_backend now doesn't compute any reassignments related to
decommission itself, we can simplify the decommission finish condition
and simply use the partition allocator to check that the node is empty.
@ztlpn ztlpn force-pushed the pb-counts-rebalancing branch from 17045c1 to 0120266 Compare June 15, 2023 11:39
@ztlpn
Copy link
Contributor Author

ztlpn commented Jun 15, 2023

/ci-repeat 2
skip-units
dt-repeat=10
tests/rptest/tests/nodes_decommissioning_test.py
tests/rptest/tests/scaling_up_test.py
tests/rptest/tests/partition_balancer_test.py
tests/rptest/tests/random_node_operations_test.py

1 similar comment
@ztlpn
Copy link
Contributor Author

ztlpn commented Jun 16, 2023

/ci-repeat 2
skip-units
dt-repeat=10
tests/rptest/tests/nodes_decommissioning_test.py
tests/rptest/tests/scaling_up_test.py
tests/rptest/tests/partition_balancer_test.py
tests/rptest/tests/random_node_operations_test.py

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.

2 participants