Re: [PATCH ha-manager v3 35/40] implement automatic rebalancing

Dominik Rusovac
2026-03-31T09:06:40Z

one nit inline

lgtm, consider this

On Mon Mar 30, 2026 at 4:30 PM CEST, Daniel Kral wrote:
> If the automatic load balancing system is enabled, it checks whether the
> cluster node imbalance exceeds some user-defined threshold for some HA
> Manager rounds ("hold duration"). If it does exceed on consecutive HA
> Manager rounds, it will choose the best resource motion to improve the
> cluster node imbalance and queue it if it significantly improves it by
> some user-defined imbalance improvement ("margin").
>
> This patch introduces resource bundles, which ensure that HA resources
> in strict positive resource affinity rules are considered as a whole
> "bundle" instead of individual HA resources.
>
> Specifically, active and stationary resource bundles are resource
> bundles, that have at least one resource running and all resources
> located on the same node. This distinction is needed as newly created
> strict positive resource affinity rules may still require some resource
> motions to enforce the rule.
>
> Additionally, the migration candidate generation prunes any target
> nodes, which do not adhere to the HA rules of these resource bundles
> before scoring these migration candidates.

nice work! also very nice idea to introduce resource bundles

>
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
> changes v2 -> v3:
> - none
>

[snip]

> diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
> index 2576c762..0f8a03a6 100644
> --- a/src/PVE/HA/Manager.pm
> +++ b/src/PVE/HA/Manager.pm
> @@ -59,10 +59,17 @@ sub new {
>
> my $self = bless {
> haenv => $haenv,
> - crs => {},
> + crs => {
> + auto_rebalance => {},
> + },
> last_rules_digest => '',
> last_groups_digest => '',
> last_services_digest => '',
> + # used to track how many HA rounds the imbalance threshold has been exceeded
> + #
> + # this is not persisted for a CRM failover as in the mean time
> + # the usage statistics might have change quite a bit already

nit:

# the usage statistics might have change[d] quite a bit already

> + sustained_imbalance_round => 0,
> group_migration_round => 3, # wait a little bit
> }, $class;
>
> @@ -97,6 +104,13 @@ sub update_crs_scheduler_mode {
> my $crs_cfg = $dc_cfg->{crs};
>
> $self->{crs}->{rebalance_on_request_start} = !!$crs_cfg->{'ha-rebalance-on-start'};
> + $self->{crs}->{auto_rebalance}->{enable} = !!$crs_cfg->{'ha-auto-rebalance'};
> + $self->{crs}->{auto_rebalance}->{threshold} = $crs_cfg->{'ha-auto-rebalance-threshold'} // 0.7;
> + $self->{crs}->{auto_rebalance}->{method} = $crs_cfg->{'ha-auto-rebalance-method'}
> + // 'bruteforce';
> + $self->{crs}->{auto_rebalance}->{hold_duration} = $crs_cfg->{'ha-auto-rebalance-hold-duration'}
> + // 3;
> + $self->{crs}->{auto_rebalance}->{margin} = $crs_cfg->{'ha-auto-rebalance-margin'} // 0.1;
>
> my $old_mode = $self->{crs}->{scheduler};
> my $new_mode = $crs_cfg->{ha} || 'basic';
> @@ -114,6 +128,150 @@ sub update_crs_scheduler_mode {
> return;
> }
>
> +# Returns a hash of lists, which contain the running, non-moving HA resource
> +# bundles, which are on the same node, implied by the strict positive resource
> +# affinity rules.
> +#
> +# Each resource bundle has a leader, which is the alphabetically first running
> +# HA resource in the resource bundle and also the key of each resource bundle
> +# in the returned hash.
> +sub get_active_stationary_resource_bundles {
> + my ($ss, $resource_affinity) = @_;
> +
> + my $resource_bundles = {};
> +OUTER: for my $sid (sort keys %$ss) {
> + # do not consider non-started resource as 'active' leading resource
> + next if $ss->{$sid}->{state} ne 'started';
> +
> + my @resources = ($sid);
> + my $nodes = { $ss->{$sid}->{node} => 1 };
> +
> + my ($dependent_resources) = get_affinitive_resources($resource_affinity, $sid);
> + if (%$dependent_resources) {
> + for my $csid (keys %$dependent_resources) {
> + next if !defined($ss->{$csid});
> + my ($state, $node) = $ss->{$csid}->@{qw(state node)};
> +
> + # do not consider stationary bundle if a dependent resource moves
> + next OUTER if $state eq 'migrate' || $state eq 'relocate';
> + # do not add non-started resource to active bundle
> + next if $state ne 'started';
> +
> + $nodes->{$node} = 1;
> +
> + push @resources, $csid;
> + }
> +
> + @resources = sort @resources;
> + }
> +
> + # skip resource bundles, which are not on the same node yet
> + next if keys %$nodes > 1;
> +
> + my $leader_sid = $resources[0];
> +
> + $resource_bundles->{$leader_sid} = \@resources;
> + }
> +
> + return $resource_bundles;
> +}
> +
> +# Returns a hash of hashes, where each item contains the resource bundle's
> +# leader, the list of HA resources in the resource bundle, and the list of
> +# possible nodes to migrate to.
> +sub get_resource_migration_candidates {
> + my ($self) = @_;
> +
> + my ($ss, $compiled_rules, $online_node_usage) =
> + $self->@{qw(ss compiled_rules online_node_usage)};
> + my ($node_affinity, $resource_affinity) =
> + $compiled_rules->@{qw(node-affinity resource-affinity)};
> +
> + my $resource_bundles = get_active_stationary_resource_bundles($ss, $resource_affinity);
> +
> + my @compact_migration_candidates = ();
> + for my $leader_sid (sort keys %$resource_bundles) {
> + my $current_leader_node = $ss->{$leader_sid}->{node};
> + my $online_nodes = { map { $_ => 1 } $online_node_usage->list_nodes() };
> +
> + my (undef, $target_nodes) = get_node_affinity($node_affinity, $leader_sid, $online_nodes);
> + my ($together, $separate) =
> + get_resource_affinity($resource_affinity, $leader_sid, $ss, $online_nodes);
> + apply_negative_resource_affinity($separate, $target_nodes);
> +
> + delete $target_nodes->{$current_leader_node};
> +
> + next if !%$target_nodes;
> +
> + push @compact_migration_candidates,
> + {
> + leader => $leader_sid,
> + nodes => [sort keys %$target_nodes],
> + resources => $resource_bundles->{$leader_sid},
> + };
> + }
> +
> + return \@compact_migration_candidates;
> +}
> +
> +sub load_balance {
> + my ($self) = @_;
> +
> + my ($crs, $haenv, $online_node_usage) = $self->@{qw(crs haenv online_node_usage)};
> + my ($auto_rebalance_opts) = $crs->{auto_rebalance};
> +
> + return if !$auto_rebalance_opts->{enable};
> + return if $crs->{scheduler} ne 'static' && $crs->{scheduler} ne 'dynamic';
> + return if $self->any_resource_motion_queued_or_running();
> +
> + my ($threshold, $method, $hold_duration, $margin) =
> + $auto_rebalance_opts->@{qw(threshold method hold_duration margin)};
> +
> + my $imbalance = $online_node_usage->calculate_node_imbalance();
> +
> + # do not load balance unless imbalance threshold has been exceeded
> + # consecutively for $hold_duration calls to load_balance()
> + if ($imbalance < $threshold) {
> + $self->{sustained_imbalance_round} = 0;
> + return;
> + } else {
> + $self->{sustained_imbalance_round}++;
> + return if $self->{sustained_imbalance_round} < $hold_duration;
> + $self->{sustained_imbalance_round} = 0;
> + }
> +
> + my $candidates = $self->get_resource_migration_candidates();
> +
> + my $result;
> + if ($method eq 'bruteforce') {
> + $result = $online_node_usage->select_best_balancing_migration($candidates);
> + } elsif ($method eq 'topsis') {
> + $result = $online_node_usage->select_best_balancing_migration_topsis($candidates);
> + }
> +
> + # happens if $candidates is empty or $method isn't handled above
> + return if !$result;
> +
> + my ($migration, $target_imbalance) = $result->@{qw(migration imbalance)};
> +
> + my $relative_change = ($imbalance - $target_imbalance) / $imbalance;
> + return if $relative_change < $margin;
> +
> + my ($sid, $source, $target) = $migration->@{qw(sid source-node target-node)};
> +
> + my (undef, $type, $id) = $haenv->parse_sid($sid);
> + my $task = $type eq 'vm' ? "migrate" : "relocate";
> + my $cmd = "$task $sid $target";
> +

nit: tbh I racked my brain a bit to get how this rounding technique
worked; so pls at least add a comment to say that you're rounding or
use printf

> + my $target_imbalance_str = int(100 * $target_imbalance + 0.5) / 100;
> + $haenv->log(
> + 'info',
> + "auto rebalance - $task $sid to $target (expected target imbalance: $target_imbalance_str)",
> + );
> +
> + $self->queue_resource_motion($cmd, $task, $sid, $target);
> +}
> +
> sub cleanup {
> my ($self) = @_;
>
> @@ -466,6 +624,21 @@ sub queue_resource_motion {
> }
> }
>
> +sub any_resource_motion_queued_or_running {
> + my ($self) = @_;
> +
> + my ($ss) = $self->@{qw(ss)};
> +
> + for my $sid (keys %$ss) {
> + my ($cmd, $state) = $ss->{$sid}->@{qw(cmd state)};
> +
> + return 1 if $state eq 'migrate' || $state eq 'relocate';
> + return 1 if defined($cmd) && ($cmd->[0] eq 'migrate' || $cmd->[0] eq 'relocate');
> + }
> +
> + return 0;
> +}
> +
> # read new crm commands and save them into crm master status
> sub update_crm_commands {
> my ($self) = @_;
> @@ -902,6 +1075,10 @@ sub manage {
> return; # disarm active and progressing, skip normal service state machine
> }
> # disarm deferred - fall through but only process services in transient states
> + } else {
> + # load balance only if disarm is disabled as during a deferred disarm
> + # the HA Manager should not introduce any new migrations
> + $self->load_balance();
> }

very thoughtful, nice

>

[snip]