[PATCH ha-manager] fix #8240: lrm: keep try count when re-queuing a waiting command

[Lucas Barros <lucasbarros.kam@gmail.com>]

Commit 65c1fba ("lrm: avoid job starvation on huge workloads") made
run_workers() prefer queued commands that were already passed over
more often, so that every resource gets its turn eventually when not
all queued commands can be started within one LRM round.

But manage_resources() queues the current command of every resource
again in each round, and queue_resource_command() replaces a queued,
not yet started command with a fresh entry with 'start_tries' reset to
0. So at the start of every round all waiting commands are equal again
and the sort falls back to the service ID: with more resources than
the LRM can handle in one round, the lexicographically first ones are
checked every round, while the others are never started and stay in
their state, e.g. 'starting', for as long as the load lasts.

Keep the try count of the replaced entry, so that commands that waited
longer are actually started first.

Signed-off-by: Lucas Barros <lucasbarros.kam@gmail.com>
---
The simulator based tests use max_workers 0, where every command is
run directly, so they do not cover this. To check it, I drove the real
manage_resources()/run_workers() of the LRM with a fake environment on
a PVE 9 node: 60 'started' services, 4 workers, each resource agent
call takes one second, and, like the CRM, a service gets a new uid
whenever the LRM reported a result for the current one. Over 4 rounds
(3 runs each):

before:
  checked at least once: 29, 31, 33 of 60
  never checked: vm:130 ... vm:160 (always the end of the sid order)

after:
  checked at least once: 60, 60, 60 of 60

The script is ~60 lines, I can post it if that helps.

Only the try count of an entry that was not started yet is carried
over; a new entry still starts at 0, and the count is dropped together
with the entry once its worker finished. If the replaced command is for
a different state, it currently inherits the count as well, as the
resource waited for its turn either way. I can restrict it to the same
state if preferred.

The series "some sid parsing and LRM speedup improvements" (Daniel
Kral, July) does not touch queue_resource_command() or the ordering,
so this should apply on top of it as well.

'make test' in src passes before and after (153 of 153 regression
tests and the 9 unit tests).
proxmox-perltidy reports no changes.

This patch was prepared with the help of an LLM based coding assistant;
I reviewed the change and ran the tests mentioned above myself.

 src/PVE/HA/LRM.pm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/PVE/HA/LRM.pm b/src/PVE/HA/LRM.pm
index 72e37e6..2105593 100644
--- a/src/PVE/HA/LRM.pm
+++ b/src/PVE/HA/LRM.pm
@@ -773,9 +773,12 @@ sub queue_resource_command {
     # with no active CRM)
     return if $state ne 'stopped' && $uid && defined($self->{results}->{$uid});
 
+    my $start_tries = 0;
     if (my $w = $self->{workers}->{$sid}) {
         return if $w->{pid}; # already started
-        # else, delete and overwrite queue entry with new command
+        # else, delete and overwrite queue entry with new command, but keep the try count, so
+        # that the fair scheduling in run_workers() still prefers it over newly queued jobs
+        $start_tries = $w->{start_tries} // 0;
         delete $self->{workers}->{$sid};
     }
 
@@ -783,7 +786,7 @@ sub queue_resource_command {
         sid => $sid,
         uid => $uid,
         state => $state,
-        start_tries => 0,
+        start_tries => $start_tries,
     };
 
     $self->{workers}->{$sid}->{params} = $params if $params;
-- 
2.47.3