public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Daniel Kral" <d.kral@proxmox.com>
To: "Michael Ryom" <Michael@RyomHerold.dk>, <pve-devel@lists.proxmox.com>
Subject: Re: [PATCH ha-manager 1/7] fix #8059: manager: auto rebalance: back off failed motions and add per-resource cooldown
Date: Wed, 14 Oct 2026 13:49:11 +0200	[thread overview]
Message-ID: <DM4JDPTYEE36.ACEUWY1RTY3C@proxmox.com> (raw)
In-Reply-To: <20260920162220.574802-2-Michael@RyomHerold.dk>

Thanks for sending in these patches!

I like the part of putting failed HA resource motions on some cooldown.

The main reasons why HA resource motions fail are because some
constraint is not encoded in appropriate node affinity rules (e.g.
storage/network device/pci device is not available on target node) or
the 'auto-rebalance' flag is not disabled for guests that cannot be
live-migrated such as a local CD-ROM image or e.g. the guest uses AMD
SEV or Intel TDX...

Most of these are inferrable by the guest config alone, so I'm working
on a patch series which automatically encodes these constraints obtained
from the guest's config files (doing that efficiently and fast for lots
of guests is the harder part). However, there will always be some
conditions that we cannot easily check just with the configuration
alone, so we need this safe guard anyway.

Generally, I'd like the exponential backoff idea, as we cannot easily
infer the cause of the failed migration (could be a temporary issue or
because of a config reason as noted above), so we need some
approximation by trial-and-error. Though I'm not sure if it's the best
fit here for what this patch is trying to fix, see below for more
information (especially the test case).

Otherwise, I'd tend to record any failed HA resource motion when it was
reported as unsuccessful by the LRM in next_state_migrate_relocate().
This way we could work with more information already, handle the case
where an HA resource bundle is moved (i.e. multiple HA resource motions)
and could use the information at other places in the future as well,
though restrict that to rebalancing migrations only. But I'm not
entirely sure on that idea yet.

The separate cooldown for previous rebalancing migrations should go in
its own patch though as it's not really related to failed resource
motions and should be reviewed in a separate patch and as a separate
idea.

On Sun Sep 20, 2026 at 6:22 PM CEST, Michael Ryom wrote:
> The automatic load balancer only refrains from issuing new motions while
> a migrate/relocate is queued or running. It never records the outcome of
> a motion it issued. If the motion fails (e.g. because the guest has a
> local CD-ROM image attached), the resource returns to 'started' on its
> source node, and the very same motion is selected again as soon as the
> hold duration has passed. This results in an endless loop of failing
> migrations, in practice about every 40 seconds with the default hold
> duration.
>
> Track the last motion issued by load_balance() and, once it is no
> longer queued or running, check whether the resource ended up on the
> target node:
>
> - on failure, back off that (resource, target) pair with an exponential
>   delay (60s, 120s, ... capped at 1h) and log a warning;
> - on success, put the resource on a cooldown (10 minutes), so that a
>   resource is not moved back and forth in quick succession, which can
>   happen when a single resource dominates the cluster load and the node
>   base loads fluctuate around each other.
>
> Backed-off targets and resources on cooldown are pruned in
> get_resource_migration_candidates() before scoring, so the balancer can
> still pick other, possibly worse but feasible, migrations.
>
> Like sustained_imbalance_round, this state is not persisted across a
> CRM failover. The delays are constants for now; they could be exposed
> as ha-auto-rebalance-* options later.
>
> The expected log of test-crs-dynamic-auto-rebalance-topsis3 changes,
> because the resource vm:105, which was moved to node2 when its load was
> high, is on cooldown when its load has gone down again and is not moved
> back immediately anymore; the balancer instead improves the imbalance
> by moving vm:106.
>
> Add two regression tests: one for the backoff of failed motions (using
> a fa: resource, which fails its first two relocations) and one for the
> cooldown suppressing an immediate qualifying reverse migration after a
> successful rebalance motion.

IMO a patch series that changes some behavior of an existing system
should add test cases in a patch before that change so it's clear in the
changing patch that it actually makes a difference and does what it
intends to do.

>
> Signed-off-by: Michael Ryom <Michael@RyomHerold.dk>
> ---
>  src/PVE/HA/Manager.pm                         | 118 ++++++++++++++++++
>  .../log.expect                                |  18 +--
>  .../test-crs-dynamic-auto-rebalance5/README   |  13 ++
>  .../test-crs-dynamic-auto-rebalance5/cmdlist  |   3 +
>  .../datacenter.cfg                            |   6 +
>  .../dynamic_service_stats                     |   6 +
>  .../hardware_status                           |   5 +
>  .../log.expect                                |  77 ++++++++++++
>  .../manager_status                            |   1 +
>  .../service_config                            |   6 +
>  .../static_service_stats                      |   6 +
>  .../test-crs-dynamic-auto-rebalance6/README   |  17 +++
>  .../test-crs-dynamic-auto-rebalance6/cmdlist  |   7 ++
>  .../datacenter.cfg                            |   6 +
>  .../dynamic_service_stats                     |   5 +
>  .../hardware_status                           |   4 +
>  .../log.expect                                |  41 ++++++
>  .../manager_status                            |   1 +
>  .../service_config                            |   5 +
>  .../static_service_stats                      |   5 +
>  20 files changed, 341 insertions(+), 9 deletions(-)
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance5/README
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance5/cmdlist
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance5/datacenter.cfg
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance5/dynamic_service_stats
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance5/hardware_status
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance5/log.expect
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance5/manager_status
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance5/service_config
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance5/static_service_stats
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance6/README
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance6/cmdlist
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance6/datacenter.cfg
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance6/dynamic_service_stats
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance6/hardware_status
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance6/log.expect
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance6/manager_status
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance6/service_config
>  create mode 100644 src/test/test-crs-dynamic-auto-rebalance6/static_service_stats
>
> diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
> index 5840a76..30b9ae2 100644
> --- a/src/PVE/HA/Manager.pm
> +++ b/src/PVE/HA/Manager.pm
> @@ -72,6 +72,15 @@ sub new {
>          # this is not persisted for a CRM failover as in the mean time
>          # the usage statistics might have change quite a bit already
>          sustained_imbalance_round => 0,
> +        # outcome tracking for resource motions issued by load_balance()
> +        #
> +        # like sustained_imbalance_round, this is not persisted for a CRM
> +        # failover
> +        auto_rebalance_state => {
> +            pending => undef, # last issued motion, until it has finished
> +            failures => {}, # "$sid:$target" => { count => $count, until => $time }
> +            cooldown => {}, # $sid => $time
> +        },
>          group_migration_round => 3, # wait a little bit
>      }, $class;
>  
> @@ -135,6 +144,92 @@ sub update_crs_scheduler_mode {
>      return;
>  }
>  
> +# delay before a failed rebalance motion to the same target node is retried,
> +# doubled for each consecutive failure up to the maximum
> +my $auto_rebalance_failure_backoff_base = 60;
> +my $auto_rebalance_failure_backoff_max = 3600;
> +# minimum time before a resource is considered for rebalancing again after it
> +# was successfully moved by the load balancer
> +my $auto_rebalance_resource_cooldown = 600;
> +
> +my $is_on_rebalance_cooldown = sub {

nit: I'd prefer `my sub name {}` over `my $name = sub {}` for this and
     the following new definitions

> +    my ($self, $sid, $now) = @_;
> +
> +    my $cooldown = $self->{auto_rebalance_state}->{cooldown};
> +    my $until = $cooldown->{$sid} // return 0;
> +
> +    if ($now >= $until) {
> +        delete $cooldown->{$sid};
> +        return 0;
> +    }
> +
> +    return 1;
> +};
> +
> +my $is_rebalance_motion_backed_off = sub {
> +    my ($self, $sid, $target, $now) = @_;
> +
> +    my $failure = $self->{auto_rebalance_state}->{failures}->{"$sid:$target"} // return 0;
> +
> +    # keep the entry after expiry, so that the failure count keeps growing if
> +    # the next attempt fails as well; it is only reset on success
> +    return $now < $failure->{until} ? 1 : 0;
> +};
> +
> +# Checks whether the last resource motion issued by load_balance() has
> +# finished and records its outcome: a per-resource cooldown on success, or an
> +# exponential backoff for the ($sid, $target) pair on failure.
> +my $check_last_rebalance_motion = sub {
> +    my ($self) = @_;
> +
> +    my ($haenv, $ss) = $self->@{qw(haenv ss)};
> +    my $state = $self->{auto_rebalance_state};
> +
> +    my $pending = $state->{pending} // return;
> +    my ($sid, $target, $task) = $pending->@{qw(sid target task)};
> +
> +    my $sd = $ss->{$sid};
> +    if (!defined($sd)) {
> +        # resource was removed in the meantime
> +        $state->{pending} = undef;
> +        return;
> +    }
> +
> +    my ($cmd, $sstate) = $sd->@{qw(cmd state)};
> +
> +    # still queued or in progress
> +    return if $sstate eq 'migrate' || $sstate eq 'relocate';
> +    return if defined($cmd) && ($cmd->[0] eq 'migrate' || $cmd->[0] eq 'relocate');

This should be factored out from the similar
any_resource_motion_queued_or_running(), e.g. as
is_resource_motion_queued_or_running($self, $sid).

> +
> +    $state->{pending} = undef;
> +
> +    my $now = $haenv->get_time();
> +    my $key = "$sid:$target";
> +
> +    if (defined($sd->{node}) && $sd->{node} eq $target) {
> +        delete $state->{failures}->{$key};
> +        $state->{cooldown}->{$sid} = $now + $auto_rebalance_resource_cooldown;
> +        return;
> +    }
> +
> +    my $failure = $state->{failures}->{$key} //= { count => 0 };

nit: use two lines for this

> +    $failure->{count}++;
> +
> +    my $delay = $auto_rebalance_failure_backoff_base * 2**($failure->{count} - 1);
> +    $delay = $auto_rebalance_failure_backoff_max
> +        if $delay > $auto_rebalance_failure_backoff_max;
> +
> +    $failure->{until} = $now + $delay;
> +
> +    $haenv->log(
> +        'warning',
> +        "auto rebalance - $task $sid to $target did not succeed"
> +            . " (attempt $failure->{count}), not retrying it for ${delay}s",
> +    );
> +
> +    return;
> +};
> +
>  # Returns a hash of lists, which contain the running, movable, non-moving HA resource
>  # bundles, which are on the same node, implied by the strict positive resource
>  # affinity rules.
> @@ -200,9 +295,12 @@ sub get_resource_migration_candidates {
>  
>      my $resource_bundles =
>          get_active_stationary_movable_resource_bundles($ss, $sc, $resource_affinity);
> +    my $now = $self->{haenv}->get_time();
>  
>      my @compact_migration_candidates = ();
>      for my $leader_sid (sort keys %$resource_bundles) {
> +        next if $is_on_rebalance_cooldown->($self, $leader_sid, $now);
> +
>          my $current_leader_node = $ss->{$leader_sid}->{node};
>          my $online_nodes = { map { $_ => 1 } $online_node_usage->list_nodes() };
>  
> @@ -213,6 +311,11 @@ sub get_resource_migration_candidates {
>  
>          delete $target_nodes->{$current_leader_node};
>  
> +        for my $node (keys %$target_nodes) {
> +            delete $target_nodes->{$node}
> +                if $is_rebalance_motion_backed_off->($self, $leader_sid, $node, $now);
> +        }
> +
>          next if !%$target_nodes;
>  
>          push @compact_migration_candidates,
> @@ -234,6 +337,10 @@ sub load_balance {
>  
>      return if !$auto_rebalance_opts->{enable};
>      return if $crs->{scheduler} ne 'static' && $crs->{scheduler} ne 'dynamic';
> +
> +    # record the outcome of the last motion issued here, if it has finished
> +    $check_last_rebalance_motion->($self);
> +
>      return if $self->any_resource_motion_queued_or_running();
>  
>      my ($threshold, $method, $hold_duration, $margin) =
> @@ -295,6 +402,17 @@ sub load_balance {
>      $haenv->log('info', "auto rebalance - $task $sid to $target ($imbalance_change_str)");
>  
>      $self->queue_resource_motion($cmd, $task, $sid, $target);
> +
> +    # queue_resource_motion() may refuse to queue the motion (e.g. blocking
> +    # resources on the target node), so only track it if it was queued
> +    if (defined($self->{ss}->{$sid}->{cmd})) {
> +        $self->{auto_rebalance_state}->{pending} = {
> +            sid => $sid,
> +            source => $source,
> +            target => $target,
> +            task => $task,
> +        };
> +    }

This does account for HA resource bundles quite implicitly, i.e., HA
resources that are migrated because of a positive resource affinity
rule, because the given migration candidates use the alphabetically
first $sid in the positive resource affinity rule as the HA resource
bundle's leader.

If a HA resource with a $sid alphabetically ordered before the former
leader's $sid was added to the positive resource affinity rule, the
backoff would just be reset.

>  }
>  
>  sub cleanup {
> diff --git a/src/test/test-crs-dynamic-auto-rebalance-topsis3/log.expect b/src/test/test-crs-dynamic-auto-rebalance-topsis3/log.expect
> index 2f19c72..dfbc904 100644
> --- a/src/test/test-crs-dynamic-auto-rebalance-topsis3/log.expect
> +++ b/src/test/test-crs-dynamic-auto-rebalance-topsis3/log.expect
> @@ -77,13 +77,13 @@ info    263    node2/lrm: service vm:103 - end migrate to node 'node3'
>  info    280    node1/crm: service 'vm:103': state changed from 'migrate' to 'started'  (node = node3)
>  info    285    node3/lrm: starting service vm:103
>  info    285    node3/lrm: service status vm:103 started
> -info    340    node1/crm: auto rebalance - migrate vm:105 to node1 (expected change for imbalance from 30.1% to 17.0%)
> -info    340    node1/crm: got crm command: migrate vm:105 node1
> -info    340    node1/crm: migrate service 'vm:105' to node 'node1'
> -info    340    node1/crm: service 'vm:105': state changed from 'started' to 'migrate'  (node = node2, target = node1)
> -info    343    node2/lrm: service vm:105 - start migrate to node 'node1'
> -info    343    node2/lrm: service vm:105 - end migrate to node 'node1'
> -info    360    node1/crm: service 'vm:105': state changed from 'migrate' to 'started'  (node = node1)
> -info    361    node1/lrm: starting service vm:105
> -info    361    node1/lrm: service status vm:105 started
> +info    340    node1/crm: auto rebalance - migrate vm:106 to node1 (expected change for imbalance from 30.1% to 20.5%)
> +info    340    node1/crm: got crm command: migrate vm:106 node1
> +info    340    node1/crm: migrate service 'vm:106' to node 'node1'
> +info    340    node1/crm: service 'vm:106': state changed from 'started' to 'migrate'  (node = node3, target = node1)
> +info    345    node3/lrm: service vm:106 - start migrate to node 'node1'
> +info    345    node3/lrm: service vm:106 - end migrate to node 'node1'
> +info    360    node1/crm: service 'vm:106': state changed from 'migrate' to 'started'  (node = node1)
> +info    361    node1/lrm: starting service vm:106
> +info    361    node1/lrm: service status vm:106 started
>  info    820     hardware: exit simulation - done
> diff --git a/src/test/test-crs-dynamic-auto-rebalance5/README b/src/test/test-crs-dynamic-auto-rebalance5/README
> new file mode 100644
> index 0000000..c24c44d
> --- /dev/null
> +++ b/src/test/test-crs-dynamic-auto-rebalance5/README
> @@ -0,0 +1,13 @@
> +Test that the auto rebalance system backs off failed rebalance motions
> +instead of retrying them indefinitely.
> +
> +The HA resource fa:102 fails its first two migrate/relocate attempts. The
> +auto rebalancer first selects fa:102 to be relocated to node2, which fails.
> +The (fa:102, node2) pair is backed off for 60 seconds, so on the next
> +balancing round the rebalancer relocates fa:102 to node3 instead, which
> +fails as well and backs off the (fa:102, node3) pair. On the third attempt,
> +the backoff for node2 has expired and the relocation to node2 succeeds.
> +
> +After the successful relocation, fa:102 is put on the per-resource cooldown,
> +so the following balancing round moves vm:102 to node3 instead to reach the
> +minimum cluster node imbalance.
> diff --git a/src/test/test-crs-dynamic-auto-rebalance5/cmdlist b/src/test/test-crs-dynamic-auto-rebalance5/cmdlist
> new file mode 100644
> index 0000000..13f90cd
> --- /dev/null
> +++ b/src/test/test-crs-dynamic-auto-rebalance5/cmdlist
> @@ -0,0 +1,3 @@
> +[
> +    [ "power node1 on", "power node2 on", "power node3 on" ]
> +]
> diff --git a/src/test/test-crs-dynamic-auto-rebalance5/datacenter.cfg b/src/test/test-crs-dynamic-auto-rebalance5/datacenter.cfg
> new file mode 100644
> index 0000000..01c8114
> --- /dev/null
> +++ b/src/test/test-crs-dynamic-auto-rebalance5/datacenter.cfg
> @@ -0,0 +1,6 @@
> +{
> +    "crs": {
> +        "ha": "dynamic",
> +        "ha-auto-rebalance": 1
> +    }
> +}
> diff --git a/src/test/test-crs-dynamic-auto-rebalance5/dynamic_service_stats b/src/test/test-crs-dynamic-auto-rebalance5/dynamic_service_stats
> new file mode 100644
> index 0000000..4eba71c
> --- /dev/null
> +++ b/src/test/test-crs-dynamic-auto-rebalance5/dynamic_service_stats
> @@ -0,0 +1,6 @@
> +{
> +    "fa:102": { "cpu": 1.0, "mem": 4294967296 },
> +    "vm:102": { "cpu": 1.0, "mem": 4294967296 },
> +    "vm:103": { "cpu": 1.0, "mem": 4294967296 },
> +    "vm:104": { "cpu": 1.0, "mem": 4294967296 }
> +}
> diff --git a/src/test/test-crs-dynamic-auto-rebalance5/hardware_status b/src/test/test-crs-dynamic-auto-rebalance5/hardware_status
> new file mode 100644
> index 0000000..ce8cf0e
> --- /dev/null
> +++ b/src/test/test-crs-dynamic-auto-rebalance5/hardware_status
> @@ -0,0 +1,5 @@
> +{
> +  "node1": { "power": "off", "network": "off", "maxcpu": 24, "maxmem": 34359738368 },
> +  "node2": { "power": "off", "network": "off", "maxcpu": 24, "maxmem": 34359738368 },
> +  "node3": { "power": "off", "network": "off", "maxcpu": 24, "maxmem": 34359738368 }
> +}
> diff --git a/src/test/test-crs-dynamic-auto-rebalance5/log.expect b/src/test/test-crs-dynamic-auto-rebalance5/log.expect
> new file mode 100644
> index 0000000..1267366
> --- /dev/null
> +++ b/src/test/test-crs-dynamic-auto-rebalance5/log.expect
> @@ -0,0 +1,77 @@
> +info      0     hardware: starting simulation
> +info     20      cmdlist: execute power node1 on
> +info     20    node1/crm: status change startup => wait_for_quorum
> +info     20    node1/lrm: status change startup => wait_for_agent_lock
> +info     20      cmdlist: execute power node2 on
> +info     20    node2/crm: status change startup => wait_for_quorum
> +info     20    node2/lrm: status change startup => wait_for_agent_lock
> +info     20      cmdlist: execute power node3 on
> +info     20    node3/crm: status change startup => wait_for_quorum
> +info     20    node3/lrm: status change startup => wait_for_agent_lock
> +info     20    node1/crm: got lock 'ha_manager_lock'
> +info     20    node1/crm: status change wait_for_quorum => master
> +info     20    node1/crm: using scheduler mode 'dynamic'
> +info     20    node1/crm: node 'node1': state changed from 'unknown' => 'online'
> +info     20    node1/crm: node 'node2': state changed from 'unknown' => 'online'
> +info     20    node1/crm: node 'node3': state changed from 'unknown' => 'online'
> +info     20    node1/crm: adding new service 'fa:102' on node 'node1'
> +info     20    node1/crm: adding new service 'vm:102' on node 'node1'
> +info     20    node1/crm: adding new service 'vm:103' on node 'node1'
> +info     20    node1/crm: adding new service 'vm:104' on node 'node1'
> +info     20    node1/crm: service 'fa:102': state changed from 'request_start' to 'started'  (node = node1)
> +info     20    node1/crm: service 'vm:102': state changed from 'request_start' to 'started'  (node = node1)
> +info     20    node1/crm: service 'vm:103': state changed from 'request_start' to 'started'  (node = node1)
> +info     20    node1/crm: service 'vm:104': state changed from 'request_start' to 'started'  (node = node1)
> +info     21    node1/lrm: got lock 'ha_agent_node1_lock'
> +info     21    node1/lrm: status change wait_for_agent_lock => active
> +info     21    node1/lrm: starting service fa:102
> +info     21    node1/lrm: service status fa:102 started
> +info     21    node1/lrm: starting service vm:102
> +info     21    node1/lrm: service status vm:102 started
> +info     21    node1/lrm: starting service vm:103
> +info     21    node1/lrm: service status vm:103 started
> +info     21    node1/lrm: starting service vm:104
> +info     21    node1/lrm: service status vm:104 started
> +info     22    node2/crm: status change wait_for_quorum => slave
> +info     24    node3/crm: status change wait_for_quorum => slave
> +info     80    node1/crm: auto rebalance - relocate fa:102 to node2 (expected change for imbalance from 100.0% to 66.1%)
> +info     80    node1/crm: got crm command: relocate fa:102 node2
> +info     80    node1/crm: relocate service 'fa:102' to node 'node2'
> +info     80    node1/crm: service 'fa:102': state changed from 'started' to 'relocate'  (node = node1, target = node2)
> +err      81    node1/lrm: service fa:102 not moved (migration error)
> +info     83    node2/lrm: got lock 'ha_agent_node2_lock'
> +info     83    node2/lrm: status change wait_for_agent_lock => active
> +err     100    node1/crm: service 'fa:102' - migration failed (exit code 1)
> +info    100    node1/crm: service 'fa:102': state changed from 'relocate' to 'started'  (node = node1)
> +warn    120    node1/crm: auto rebalance - relocate fa:102 to node2 did not succeed (attempt 1), not retrying it for 60s
> +info    160    node1/crm: auto rebalance - relocate fa:102 to node3 (expected change for imbalance from 100.0% to 66.1%)
> +info    160    node1/crm: got crm command: relocate fa:102 node3
> +info    160    node1/crm: relocate service 'fa:102' to node 'node3'
> +info    160    node1/crm: service 'fa:102': state changed from 'started' to 'relocate'  (node = node1, target = node3)
> +err     161    node1/lrm: service fa:102 not moved (migration error)
> +info    165    node3/lrm: got lock 'ha_agent_node3_lock'
> +info    165    node3/lrm: status change wait_for_agent_lock => active
> +err     180    node1/crm: service 'fa:102' - migration failed (exit code 1)
> +info    180    node1/crm: service 'fa:102': state changed from 'relocate' to 'started'  (node = node1)
> +warn    200    node1/crm: auto rebalance - relocate fa:102 to node3 did not succeed (attempt 1), not retrying it for 60s
> +info    240    node1/crm: auto rebalance - relocate fa:102 to node2 (expected change for imbalance from 100.0% to 66.1%)
> +info    240    node1/crm: got crm command: relocate fa:102 node2
> +info    240    node1/crm: relocate service 'fa:102' to node 'node2'
> +info    240    node1/crm: service 'fa:102': state changed from 'started' to 'relocate'  (node = node1, target = node2)
> +info    241    node1/lrm: service fa:102 - start relocate to node 'node2'
> +info    241    node1/lrm: stopping service fa:102 (relocate)
> +info    241    node1/lrm: service status fa:102 stopped
> +info    241    node1/lrm: service fa:102 - end relocate to node 'node2'
> +info    260    node1/crm: service 'fa:102': state changed from 'relocate' to 'started'  (node = node2)
> +info    263    node2/lrm: starting service fa:102
> +info    263    node2/lrm: service status fa:102 started

Well, this is a perculiar test case, because the only reason this will
result in doing a successful rebalancing migration is because the third
digit (2) in fa:102 specifies that 2 migration attempts will fail and
the 3rd will be successful.

If we change the test case, e.g. use fa:109, this will end up in a
short-lived cycle, where load_balance() will try

1) fa:109 -> node2
2) fa:109 -> node3
3) fa:109 -> node2
4) fa:109 -> node3
5) fa:109 -> node2
6) fa:109 -> node3
7) vm:102 -> node2

This lies in the fact that with the default setting of 30 seconds in
hold duration and a base backoff time of 60 seconds, for the first few
migration attempts the backoff duration of fa:109 -> node2 will be over
right when fa:109 -> node3 fails, so they will cycle until the backoff
duration is long enough for both that they will overlap once so that
actually the third-best rebalancing migration (vm:102 -> node2) will be
tried.

This makes me believe that the exponential backoff isn't solving this
problem well, i.e., to not retry failing rebalancing migrations
repeatedly in the same series of trying out a rebalancing migration.

I think it would be better to just remove the migration candidates that
failed in previous attempts in the same series of trying out a
rebalancing migration alltogether and only retry them in another series
of rebalancing migrations.

Maybe the exponential backoff solution could improve the situation more
globally, that is across multiple rebalancing migration "series/rounds",
so that the information about failed migration attempts is persisted.
That is:

1) We need to rebalance (imbalance > threshold for hold duration)
2) Best migration: vm:100 -> node2
3) vm:100 -> node2 fails
4) Best migration: vm:103 -> node1
5) vm:103 -> node1 succeeds
... some time later ...
6) We need to rebalance again

vm:100 -> node2 would be the best migration again, but since it failed
in the last rebalancing attempt, leave it out here but try again in the
next rebalancing attempt; so try the next-best migration candidate

In the following rebalancing attempt, vm:100 -> node2 can be tried
again, but if it fails again, then it's backed off for the next 2
rebalancing migration attempts.

> +info    320    node1/crm: auto rebalance - migrate vm:102 to node3 (expected change for imbalance from 66.1% to 25.0%)
> +info    320    node1/crm: got crm command: migrate vm:102 node3
> +info    320    node1/crm: migrate service 'vm:102' to node 'node3'
> +info    320    node1/crm: service 'vm:102': state changed from 'started' to 'migrate'  (node = node1, target = node3)
> +info    321    node1/lrm: service vm:102 - start migrate to node 'node3'
> +info    321    node1/lrm: service vm:102 - end migrate to node 'node3'
> +info    340    node1/crm: service 'vm:102': state changed from 'migrate' to 'started'  (node = node3)
> +info    345    node3/lrm: starting service vm:102
> +info    345    node3/lrm: service status vm:102 started
> +info    620     hardware: exit simulation - done
> diff --git a/src/test/test-crs-dynamic-auto-rebalance5/manager_status b/src/test/test-crs-dynamic-auto-rebalance5/manager_status
> new file mode 100644
> index 0000000..0967ef4
> --- /dev/null
> +++ b/src/test/test-crs-dynamic-auto-rebalance5/manager_status
> @@ -0,0 +1 @@
> +{}
> diff --git a/src/test/test-crs-dynamic-auto-rebalance5/service_config b/src/test/test-crs-dynamic-auto-rebalance5/service_config
> new file mode 100644
> index 0000000..6c4ad4b
> --- /dev/null
> +++ b/src/test/test-crs-dynamic-auto-rebalance5/service_config
> @@ -0,0 +1,6 @@
> +{
> +    "fa:102": { "node": "node1", "state": "started" },
> +    "vm:102": { "node": "node1", "state": "started" },
> +    "vm:103": { "node": "node1", "state": "started" },
> +    "vm:104": { "node": "node1", "state": "started" }
> +}
> diff --git a/src/test/test-crs-dynamic-auto-rebalance5/static_service_stats b/src/test/test-crs-dynamic-auto-rebalance5/static_service_stats
> new file mode 100644
> index 0000000..41dc446
> --- /dev/null
> +++ b/src/test/test-crs-dynamic-auto-rebalance5/static_service_stats
> @@ -0,0 +1,6 @@
> +{
> +    "fa:102": { "maxcpu": 2.0, "maxmem": 8589934592 },
> +    "vm:102": { "maxcpu": 2.0, "maxmem": 8589934592 },
> +    "vm:103": { "maxcpu": 2.0, "maxmem": 8589934592 },
> +    "vm:104": { "maxcpu": 2.0, "maxmem": 8589934592 }
> +}




  reply	other threads:[~2026-10-14 11:49 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-20 16:22 [PATCH ha-manager 0/7] auto rebalance: fix failure retry loop, oscillation and idle-cluster churn Michael Ryom
2026-09-20 16:22 ` [PATCH ha-manager 1/7] fix #8059: manager: auto rebalance: back off failed motions and add per-resource cooldown Michael Ryom
2026-10-14 11:49   ` Daniel Kral [this message]
2026-09-20 16:22 ` [PATCH ha-manager 2/7] env: dynamic service stats: use host-side memory footprint of guests Michael Ryom
2026-09-21  9:18   ` Dominik Rusovac
2026-09-20 16:22 ` [PATCH ha-manager 3/7] manager: auto rebalance: require a minimum absolute imbalance improvement Michael Ryom
2026-09-22  6:18   ` Dominik Rusovac
2026-09-22 10:09     ` Michael Ryom
2026-10-14 12:23   ` Daniel Kral
2026-10-14 13:03     ` Dominik Rusovac
2026-09-20 16:22 ` [PATCH ha-manager 4/7] sim: hardware: report actual running state in cluster service stats Michael Ryom
2026-09-20 16:22 ` [PATCH ha-manager 5/7] sim: hardware: allow setting a base load for nodes Michael Ryom
2026-09-20 16:22 ` [PATCH ha-manager 6/7] usage: dynamic: smooth the unaccounted node load Michael Ryom
2026-09-20 16:22 ` [PATCH ha-manager 7/7] manager: auto rebalance: only balance under actual node resource pressure Michael Ryom
2026-10-14 14:29   ` Daniel Kral

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM4JDPTYEE36.ACEUWY1RTY3C@proxmox.com \
    --to=d.kral@proxmox.com \
    --cc=Michael@RyomHerold.dk \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal