From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id DE8501FF138 for ; Mon, 20 Jul 2026 14:39:45 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 57592216BC; Mon, 20 Jul 2026 14:38:17 +0200 (CEST) From: Daniel Kral To: pve-devel@lists.proxmox.com Subject: [PATCH-SERIES ha-manager 00/16] some sid parsing and LRM speedup improvements Date: Mon, 20 Jul 2026 14:36:49 +0200 Message-ID: <20260720123705.216089-1-d.kral@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784551005665 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.177 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_ASCII_DIVIDERS 0.8 Email that uses ascii formatting dividers and possible spam tricks KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: LBZAUZPV2DHV6QG35HMZANMO2G5YEFV4 X-Message-ID-Hash: LBZAUZPV2DHV6QG35HMZANMO2G5YEFV4 X-MailFrom: d.kral@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: It was reported in the enterprise support, that nodes with high counts of HA resources (250+ HA resources) cause the pve-ha-lrm daemon to consumes quite a lot of CPU time while being idle. Furthermore, migration commands for HA resources might take quite a long time to be actually committed when the max_workers value is not high enough. This smaller patch series tries to mitigate the former issue first, which consists of making some performance improvements for the hot LRM code paths, which consumed the most resources: - the service config is read and validated very often - because of that, parse_sid() is called many times as well - other smaller loops are redundant and are moved out of the hot paths I started out with making parse_sid() a little lighter, though because of the latter changes the performance bottleneck of parse_sid() became rather meaningless. I left the refactoring and changes in though as having a stricter separation where both VMIDs and SIDs can be used and where only SIDs can be used is a little nicer now. The patch series consists of: PATCH 1-2 make tidy + small cleanup PATCH 3-9 refactoring and splitting up parse_sid() and its users PATCH 10-12 prep work to add documentation and rename some methods PATCH 13 reading the manager status only once per work iter PATCH 14 reading the service config only once per work iter PATCH 15-16 factoring out loops which are invariant per work iter In a test startup of 768 HA resources on a single physical host at the same time, profiling with Devel::NYTProf 6.14 [0] showed the following: +---------------------------+---------------+--------------+ | Subroutine | Before Change | After Change | +---------------------------+---------------+--------------+ | check_active_workers | 327ms | 998µs | | resource_command_finished | 22.7ms | 50µs | | handle_service_exitcode | 21.1ms | 12µs | +---------------------------+---------------+--------------+ Considering that these are called quite often per LRM cycle, these drops in execution time per call are significant. Proposal for LRM worker loop improvements ----------------------------------------- However, this patch series does only indirectly improve the situation where HA resources' migration commands (especially if these are lexicopgraphically at the bottom end) take a lot of time as this is still pretty much capped by max_workers value even with the nice work by @Thomas to make the sorting of the worker execution fairer [1]. Currently, the LRM does queue resource commands for the following states of the HA resources: (1) HA resource is in 'started' state (2) HA resource is in 'request_stop'/'stopped' state (3) HA resource is in 'migrate'/'relocate' state (4) HA resource is in 'error' state For each of those HA resource in such a state, a worker is queued to be run *once*. However, for the started HA resources (1), these workers are queued in each HA Manager round. So if there are 768 running HA resources on a single node, there are 768 workers to check whether the HA resource is still running. The datacenter config's $max_workers does limit how many HA resource workers we can run at a single time. We give these LRM workers 8 seconds in these $max_workers slots while we wait 1 second each time to waitpid() them again. This might limit the amount of tasks that can be processed per LRM cycle severly. Therefore, I propose to implement a work-stealing scheduling scheme with a reusable worker thread pool for this, as this would allow the threads to work on their tasks as soon as they finished the former instead of exiting and waitpid()'ing on them in the main thread. Perl has a threads implementation [2] and I'd use the thread-safe Thread::Queue module [3] in both directions so that the main thread can queue the resource commands on there and the worker threads can queue the results back to the main thread. However, as I don't have much experience with Perl threads and as it's discouraged because of its heavy weight as noted in its documentation [2], I'd like some more input on this whether it's a good idea. Though it seems that the larger part of it being discouraged is because of its heavy weight, this wouldn't be much of a problem as this proposal uses reusable worker threads instead of creating new ones repeatedly. [0] https://metacpan.org/pod/Devel::NYTProf [1] https://git.proxmox.com/?p=pve-ha-manager.git;a=commitdiff;h=65c1fbac992d8a1f26c401cf49f2d4848bc42080 [2] https://perldoc.perl.org/threads [3] https://perldoc.perl.org/Thread::Queue Daniel Kral (16): make tidy resources: remove commented ipaddr resource type tree-wide: use the term vmid instead of name when referring to VMs/CTs api: resources: remove unused return values at parse_sid callsites make parse_sid always return an array config: use early returns in parse_sid config: make update_single_resource_config_inplace only allow sids introduce separate parse_vmid_or_sid helper subroutine drop the unmodified sid return array entry value from parse_sid lrm: document and initialize LRM instance properties lrm: rename update_lrm_status to flush_lrm_status lrm: rename update_service_status to update_lrm_status lrm: update the service status once per work iteration lrm: read service config once per work iteration lrm: compute valid service uids hash set once per work iteration lrm: prune non-existent HA resources from results once per work iteration src/PVE/API2/HA/Resources.pm | 17 ++--- src/PVE/CLI/ha_manager.pm | 6 +- src/PVE/HA/Config.pm | 42 +++++------ src/PVE/HA/Env/PVE2.pm | 6 +- src/PVE/HA/LRM.pm | 85 ++++++++++++++--------- src/PVE/HA/Manager.pm | 6 +- src/PVE/HA/Resources.pm | 38 ++-------- src/PVE/HA/Resources/PVECT.pm | 4 +- src/PVE/HA/Resources/PVEVM.pm | 4 +- src/PVE/HA/Sim/Env.pm | 4 +- src/test/test-cfs-unavailable1/log.expect | 5 -- src/test/test-group-migrate4/log.expect | 10 --- 12 files changed, 104 insertions(+), 123 deletions(-) -- 2.47.3