[PATCH ha-manager 7/7] manager: auto rebalance: only balance under actual node resource pressure

[Michael Ryom <Michael@RyomHerold.dk>]

The load balancer acted on load asymmetry alone. But asymmetric node
loads are not, by themselves, a performance problem: on an almost idle
cluster, the imbalance metric (a coefficient of variation, std/mean)
becomes hypersensitive, and observed live, a barely-qualifying
migration (60.4% -> 54.0% expected imbalance) was issued at 2-7% node
load, with nothing to gain for any workload. The purpose of the load
balancer is better performance for the guests, not symmetric load
figures.

Gate the balancer on the pressure stall information (PSI) that the
nodes already broadcast in their RRD stats: only balance while some
node reports a cpu or memory some-pressure (avg10) of at least 10%,
i.e. work on that node actually loses a meaningful share of its time
waiting on resources and could run better with more headroom. Unlike a
load threshold, this criterion is independent of the cluster's size and
load level, and it directly expresses the question "would anything
perform better elsewhere?" - akin to the contention-driven (rather than
load-driven) approach of other schedulers like VMware DRS.

Deliberately only the node pressure is considered, although the guests'
own pressure stats are collected as well: a guest saturating its own
vCPUs or its own memory reports pressure too, but gets neither more
cores nor more memory by being migrated, while host-caused starvation
(contention with other guests or host processes) shows up in the node
pressure - and only that kind can be improved by a migration.

The kernel's avg10 pressure values are already exponentially averaged
over ten seconds, so no additional smoothing is applied. IO pressure is
deliberately not considered for now, since with shared storage it
usually cannot be improved by a migration. If no pressure information
is available at all (e.g. nodes still broadcasting an older stats
schema), balancing proceeds as before.

The minimum pressure is a constant for now; like the other balancer
constants, it could be exposed as a ha-auto-rebalance-* option later.

Signed-off-by: Michael Ryom <Michael@RyomHerold.dk>
---
 src/PVE/HA/Env/PVE2.pm                        | 25 +++++++++++++
 src/PVE/HA/Manager.pm                         | 16 +++++++++
 src/PVE/HA/Sim/Hardware.pm                    | 16 +++++++--
 src/PVE/HA/Usage.pm                           |  9 +++++
 src/PVE/HA/Usage/Dynamic.pm                   | 23 ++++++++++++
 .../test-crs-dynamic-auto-rebalance9/README   | 15 ++++++++
 .../test-crs-dynamic-auto-rebalance9/cmdlist  |  4 +++
 .../datacenter.cfg                            |  6 ++++
 .../dynamic_service_stats                     |  4 +++
 .../hardware_status                           |  4 +++
 .../log.expect                                | 36 +++++++++++++++++++
 .../manager_status                            |  1 +
 .../service_config                            |  4 +++
 .../static_service_stats                      |  4 +++
 14 files changed, 165 insertions(+), 2 deletions(-)
 create mode 100644 src/test/test-crs-dynamic-auto-rebalance9/README
 create mode 100644 src/test/test-crs-dynamic-auto-rebalance9/cmdlist
 create mode 100644 src/test/test-crs-dynamic-auto-rebalance9/datacenter.cfg
 create mode 100644 src/test/test-crs-dynamic-auto-rebalance9/dynamic_service_stats
 create mode 100644 src/test/test-crs-dynamic-auto-rebalance9/hardware_status
 create mode 100644 src/test/test-crs-dynamic-auto-rebalance9/log.expect
 create mode 100644 src/test/test-crs-dynamic-auto-rebalance9/manager_status
 create mode 100644 src/test/test-crs-dynamic-auto-rebalance9/service_config
 create mode 100644 src/test/test-crs-dynamic-auto-rebalance9/static_service_stats

diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm
index 8c2b03d..7d7fceb 100644
--- a/src/PVE/HA/Env/PVE2.pm
+++ b/src/PVE/HA/Env/PVE2.pm
@@ -47,6 +47,8 @@ use constant {
     RRD_VM_INDEX_MAXMEM => 7,
     RRD_VM_INDEX_MEM => 8,
     RRD_VM_INDEX_MEMHOST => 15,
+    RRD_VM_INDEX_PRESSURE_CPU_SOME => 16,
+    RRD_VM_INDEX_PRESSURE_MEM_SOME => 20,
 };
 
 # rrd entry indices for PVE nodes
@@ -56,8 +58,21 @@ use constant {
     RRD_NODE_INDEX_CPU => 5,
     RRD_NODE_INDEX_MAXMEM => 7,
     RRD_NODE_INDEX_MEM => 8,
+    RRD_NODE_INDEX_PRESSURE_CPU_SOME => 17,
+    RRD_NODE_INDEX_PRESSURE_MEM_SOME => 20,
 };
 
+# returns the numeric value of the given RRD column, or undef if the column
+# is not present (e.g. nodes broadcasting an older schema)
+my sub rrd_column_or_undef {
+    my ($rrdentry, $index) = @_;
+
+    my $value = $rrdentry->[$index];
+    return undef if !defined($value) || $value eq '' || $value eq 'U';
+
+    return $value + 0.0;
+}
+
 my $HOSTNAME_RE = qr/(?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]{,61}?[a-zA-Z0-9])?)/;
 
 sub new {
@@ -615,6 +630,11 @@ sub get_dynamic_service_stats {
             cpu => (($rrdentry->[RRD_VM_INDEX_CPU] || 0.0) + 0.0) * $maxcpu,
             maxmem => int($rrdentry->[RRD_VM_INDEX_MAXMEM] || 0),
             mem => $mem,
+            # PSI avg10 in percent; undef if the guest broadcasts an older
+            # schema without pressure information
+            pressurecpusome => rrd_column_or_undef($rrdentry, RRD_VM_INDEX_PRESSURE_CPU_SOME),
+            pressurememorysome =>
+                rrd_column_or_undef($rrdentry, RRD_VM_INDEX_PRESSURE_MEM_SOME),
         };
     }
 
@@ -660,6 +680,11 @@ sub get_dynamic_node_stats {
             cpu => (($rrdentry->[RRD_NODE_INDEX_CPU] || 0.0) + 0.0) * $maxcpu,
             maxmem => int($rrdentry->[RRD_NODE_INDEX_MAXMEM] || 0),
             mem => int($rrdentry->[RRD_NODE_INDEX_MEM] || 0),
+            # PSI avg10 in percent; undef if the node broadcasts an older
+            # schema without pressure information
+            pressurecpusome => rrd_column_or_undef($rrdentry, RRD_NODE_INDEX_PRESSURE_CPU_SOME),
+            pressurememorysome =>
+                rrd_column_or_undef($rrdentry, RRD_NODE_INDEX_PRESSURE_MEM_SOME),
         };
     }
 
diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 6e1827f..c34da89 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -155,6 +155,15 @@ 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;
+# minimum pressure stall value (PSI avg10, in percent) among the nodes for
+# the load balancer to act at all: load asymmetry by itself is not a
+# performance problem, and the node pressure stall information directly
+# measures which share of its time work on the node spends waiting on
+# resources - i.e. whether anything could actually run better with more
+# headroom - independent of the cluster's size or load level; if no pressure
+# information is available at all (older stats schema), balancing proceeds
+# as before
+my $auto_rebalance_min_pressure = 10.0;
 # minimum absolute improvement of the node imbalance for a rebalance motion;
 # the relative margin alone provides almost no hysteresis when a single
 # resource dominates the cluster load: with two nodes, a resource load v and
@@ -368,6 +377,13 @@ sub load_balance {
         return;
     }
 
+    # nothing to gain from balancing while no workload is short on resources
+    my $max_pressure = $online_node_usage->max_pressure();
+    if (defined($max_pressure) && $max_pressure < $auto_rebalance_min_pressure) {
+        $self->{sustained_imbalance_round} = 0;
+        return;
+    }
+
     my $imbalance = $online_node_usage->calculate_node_imbalance();
 
     # do not load balance unless imbalance threshold has been exceeded
diff --git a/src/PVE/HA/Sim/Hardware.pm b/src/PVE/HA/Sim/Hardware.pm
index aa1a886..8766a6f 100644
--- a/src/PVE/HA/Sim/Hardware.pm
+++ b/src/PVE/HA/Sim/Hardware.pm
@@ -791,7 +791,8 @@ sub get_cfs_state {
 #   crm enable-node-maintenance <node>
 #   crm disable-node-maintenance <node>
 #   pve-manager-version <node> set <version>   # note: this is NOT the *ha*-manager version
-#   node <node> set-dynamic-stats [cpu <cores>] [mem <MiB>]  # load outside HA services
+#   node <node> set-dynamic-stats [cpu <cores>] [mem <MiB>] [pressurecpusome <pct>] [pressurememorysome <pct>]
+#     (load and pressure outside of the HA-managed services)
 #   reboot <node>
 #   shutdown <node>
 #   restart-lrm <node>
@@ -888,7 +889,12 @@ sub sim_hardware_cmd {
             die "sim_hardware_cmd: missing target stat for '$action' command"
                 if !@params;
 
-            my $conversions = { cpu => sub { 0.0 + $_[0] }, mem => sub { $_[0] * 1024**2 } };
+            my $conversions = {
+                cpu => sub { 0.0 + $_[0] },
+                mem => sub { $_[0] * 1024**2 },
+                pressurecpusome => sub { 0.0 + $_[0] },
+                pressurememorysome => sub { 0.0 + $_[0] },
+            };
 
             for my ($target, $val) (@params) {
                 die "sim_hardware_cmd: missing value for '$action $target' command"
@@ -1312,6 +1318,12 @@ sub get_dynamic_node_stats {
         $stats->{$node}->{cpu} = $cstatus->{$node}->{cpu} // 0.0;
         $stats->{$node}->{maxmem} = $stats->{$node}->{maxmem} // $default_node_maxmem;
         $stats->{$node}->{mem} = $cstatus->{$node}->{mem} // 0;
+        # PSI avg10 in percent; keep undef (pressure information not
+        # available) distinct from an explicit zero (no pressure)
+        for my $pressure (qw(pressurecpusome pressurememorysome)) {
+            $stats->{$node}->{$pressure} = $cstatus->{$node}->{$pressure}
+                if defined($cstatus->{$node}->{$pressure});
+        }
     }
 
     my $service_conf = $self->read_service_config();
diff --git a/src/PVE/HA/Usage.pm b/src/PVE/HA/Usage.pm
index 659ab30..e0890fb 100644
--- a/src/PVE/HA/Usage.pm
+++ b/src/PVE/HA/Usage.pm
@@ -66,6 +66,15 @@ sub calculate_node_imbalance {
     die "implement in subclass";
 }
 
+# Returns the highest pressure stall value (PSI avg10, in percent) among the
+# nodes, or undef if the usage implementation has no notion of pressure
+# (e.g. basic or static) or no node reported pressure information.
+sub max_pressure {
+    my ($self) = @_;
+
+    return undef;
+}
+
 sub score_best_balancing_migrations {
     my ($self, $migration_candidates, $limit) = @_;
 
diff --git a/src/PVE/HA/Usage/Dynamic.pm b/src/PVE/HA/Usage/Dynamic.pm
index fc88ddf..b2ed79a 100644
--- a/src/PVE/HA/Usage/Dynamic.pm
+++ b/src/PVE/HA/Usage/Dynamic.pm
@@ -184,6 +184,29 @@ sub calculate_node_imbalance {
     return $node_imbalance // 0.0;
 }
 
+# NOTE only the *node* pressure is considered: a guest saturating its own
+# vCPUs or its own memory reports pressure too, but gets neither more cores
+# nor more memory by being migrated, while starvation caused by the host
+# (contention with other guests or host processes) shows up in the node's
+# pressure - and only that kind can be improved by a migration
+sub max_pressure {
+    my ($self) = @_;
+
+    my $max_pressure;
+
+    for my $node ($self->list_nodes()) {
+        my $stats = $self->{'node-stats'}->{$node} or next;
+
+        for my $pressure ($stats->@{qw(pressurecpusome pressurememorysome)}) {
+            next if !defined($pressure);
+            $max_pressure = $pressure
+                if !defined($max_pressure) || $pressure > $max_pressure;
+        }
+    }
+
+    return $max_pressure;
+}
+
 sub score_best_balancing_migrations {
     my ($self, $migration_candidates, $limit) = @_;
 
diff --git a/src/test/test-crs-dynamic-auto-rebalance9/README b/src/test/test-crs-dynamic-auto-rebalance9/README
new file mode 100644
index 0000000..d19bd97
--- /dev/null
+++ b/src/test/test-crs-dynamic-auto-rebalance9/README
@@ -0,0 +1,15 @@
+Test that the auto rebalance system does not act while no node or HA-managed
+service reports meaningful resource pressure, even though the relative
+imbalance is high and a strongly qualifying rebalance motion exists.
+
+The cluster has two nodes with all load on node1: a HA resource excluded
+from auto rebalancing (vm:101) and a movable HA resource vm:100. The
+imbalance is 100% and migrating vm:100 to the empty node2 would improve it
+far beyond the margin and the minimum absolute improvement. But both nodes
+report zero pressure stall values, i.e. no workload spends any time waiting
+on resources, so there is no performance problem a migration could solve and
+no motion may be issued - regardless of node1's 30% load.
+
+Then node1 reports a CPU pressure of 25%, i.e. some workload now loses a
+quarter of its time waiting for CPU, and the pending migration of vm:100 to
+node2 must be carried out.
diff --git a/src/test/test-crs-dynamic-auto-rebalance9/cmdlist b/src/test/test-crs-dynamic-auto-rebalance9/cmdlist
new file mode 100644
index 0000000..3cb6c2a
--- /dev/null
+++ b/src/test/test-crs-dynamic-auto-rebalance9/cmdlist
@@ -0,0 +1,4 @@
+[
+    [ "power node1 on", "power node2 on" ],
+    [ "node node1 set-dynamic-stats pressurecpusome 25" ]
+]
diff --git a/src/test/test-crs-dynamic-auto-rebalance9/datacenter.cfg b/src/test/test-crs-dynamic-auto-rebalance9/datacenter.cfg
new file mode 100644
index 0000000..01c8114
--- /dev/null
+++ b/src/test/test-crs-dynamic-auto-rebalance9/datacenter.cfg
@@ -0,0 +1,6 @@
+{
+    "crs": {
+        "ha": "dynamic",
+        "ha-auto-rebalance": 1
+    }
+}
diff --git a/src/test/test-crs-dynamic-auto-rebalance9/dynamic_service_stats b/src/test/test-crs-dynamic-auto-rebalance9/dynamic_service_stats
new file mode 100644
index 0000000..7705333
--- /dev/null
+++ b/src/test/test-crs-dynamic-auto-rebalance9/dynamic_service_stats
@@ -0,0 +1,4 @@
+{
+    "vm:100": { "cpu": 9.6, "mem": 0 },
+    "vm:101": { "cpu": 4.8, "mem": 0 }
+}
diff --git a/src/test/test-crs-dynamic-auto-rebalance9/hardware_status b/src/test/test-crs-dynamic-auto-rebalance9/hardware_status
new file mode 100644
index 0000000..eeef22a
--- /dev/null
+++ b/src/test/test-crs-dynamic-auto-rebalance9/hardware_status
@@ -0,0 +1,4 @@
+{
+  "node1": { "power": "off", "network": "off", "maxcpu": 24, "maxmem": 34359738368, "pressurecpusome": 0.0, "pressurememorysome": 0.0 },
+  "node2": { "power": "off", "network": "off", "maxcpu": 24, "maxmem": 34359738368, "pressurecpusome": 0.0, "pressurememorysome": 0.0 }
+}
diff --git a/src/test/test-crs-dynamic-auto-rebalance9/log.expect b/src/test/test-crs-dynamic-auto-rebalance9/log.expect
new file mode 100644
index 0000000..aed4165
--- /dev/null
+++ b/src/test/test-crs-dynamic-auto-rebalance9/log.expect
@@ -0,0 +1,36 @@
+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    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: adding new service 'vm:100' on node 'node1'
+info     20    node1/crm: adding new service 'vm:101' on node 'node1'
+info     20    node1/crm: service 'vm:100': state changed from 'request_start' to 'started'  (node = node1)
+info     20    node1/crm: service 'vm:101': 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 vm:100
+info     21    node1/lrm: service status vm:100 started
+info     21    node1/lrm: starting service vm:101
+info     21    node1/lrm: service status vm:101 started
+info     22    node2/crm: status change wait_for_quorum => slave
+info    120      cmdlist: execute node node1 set-dynamic-stats pressurecpusome 25
+info    160    node1/crm: auto rebalance - migrate vm:100 to node2 (expected change for imbalance from 100.0% to 33.3%)
+info    160    node1/crm: got crm command: migrate vm:100 node2
+info    160    node1/crm: migrate service 'vm:100' to node 'node2'
+info    160    node1/crm: service 'vm:100': state changed from 'started' to 'migrate'  (node = node1, target = node2)
+info    161    node1/lrm: service vm:100 - start migrate to node 'node2'
+info    161    node1/lrm: service vm:100 - end migrate to node 'node2'
+info    163    node2/lrm: got lock 'ha_agent_node2_lock'
+info    163    node2/lrm: status change wait_for_agent_lock => active
+info    180    node1/crm: service 'vm:100': state changed from 'migrate' to 'started'  (node = node2)
+info    183    node2/lrm: starting service vm:100
+info    183    node2/lrm: service status vm:100 started
+info    720     hardware: exit simulation - done
diff --git a/src/test/test-crs-dynamic-auto-rebalance9/manager_status b/src/test/test-crs-dynamic-auto-rebalance9/manager_status
new file mode 100644
index 0000000..0967ef4
--- /dev/null
+++ b/src/test/test-crs-dynamic-auto-rebalance9/manager_status
@@ -0,0 +1 @@
+{}
diff --git a/src/test/test-crs-dynamic-auto-rebalance9/service_config b/src/test/test-crs-dynamic-auto-rebalance9/service_config
new file mode 100644
index 0000000..3f12542
--- /dev/null
+++ b/src/test/test-crs-dynamic-auto-rebalance9/service_config
@@ -0,0 +1,4 @@
+{
+    "vm:100": { "node": "node1", "state": "started" },
+    "vm:101": { "node": "node1", "state": "started", "auto-rebalance": 0 }
+}
diff --git a/src/test/test-crs-dynamic-auto-rebalance9/static_service_stats b/src/test/test-crs-dynamic-auto-rebalance9/static_service_stats
new file mode 100644
index 0000000..0dfad75
--- /dev/null
+++ b/src/test/test-crs-dynamic-auto-rebalance9/static_service_stats
@@ -0,0 +1,4 @@
+{
+    "vm:100": { "maxcpu": 12.0, "maxmem": 8589934592 },
+    "vm:101": { "maxcpu": 6.0, "maxmem": 8589934592 }
+}
-- 
2.47.3