From: "Daniel Kral" <d.kral@proxmox.com>
To: "Thomas Lamprecht" <t.lamprecht@proxmox.com>,
"Proxmox VE development discussion" <pve-devel@lists.proxmox.com>,
"Daniel Kral" <d.kral@proxmox.com>
Subject: Re: [pve-devel] [PATCH ha-manager v2 05/18] rules: add merged positive resource affinity info in global checks
Date: Fri, 31 Oct 2025 11:01:23 +0100 [thread overview]
Message-ID: <DDWF5LB1YXU2.1AQWDK136FJFE@proxmox.com> (raw)
In-Reply-To: <9fc563b3-6df0-4f9e-80df-6c1834073c33@proxmox.com>
On Wed Sep 10, 2025 at 7:35 PM CEST, Thomas Lamprecht wrote:
> Am 09.09.25 um 10:36 schrieb Daniel Kral:
>> The node affinity and positive resource affinity rule subset is checked
>> whether the HA resources in a positive resource affinity rule are in
>> more than one node affinity rule in total.
>>
>> This check has the assumption that each positive resource affinity
>> rule's resource set is disjoint from each other, but this is only done
>> in the later transformation stage when positive resource affinity with
>> overlapping HA resources in them are merged to one rule.
>>
>> For example, the following inconsistent rules are not pruned:
>>
>> - positive resource affinity rule between vm:101 and vm:102
>> - positive resource affinity rule between vm:102 and vm:103
>> - node affinity rule for vm:101 on node1
>> - node affinity rule for vm:103 on node3
>
> This is only a real problem if both node affinity rules are configured
> to be strict. Your test case (and FWICT code) acts that way, so mostly
> relevant for the commit message to avoid potential confusion about what
> rules get/needs to be pruned. Can be improved on applying though, no need
> for a v3 just for that, just wanted to note it to avoid forgetting it in
> case I do not get around to finish review here soonish.
I assumed this to be true too when I read it in September, but as I
reviewed this again for sending a new revision for this series now I
noticed that it also prunes non-strict node affinity rules.
We follow the priority classes quite strictly for node affinity rules/HA
groups (for both non-strict and strict ones) with only respecting the
highest nodes in the highest priority class. As the non-member nodes for
non-strict node affinity rules are added with priority -1, it depends on
whether none of the higher priority nodes are online and so we cannot
verify here whether this will be the case.
A more revealing example would be a cluster with the 3 nodes node1,
node2, and node3 and the following rules (based on the example above):
- positive resource affinity rule between vm:101 and vm:102
- positive resource affinity rule between vm:102 and vm:103
- non-strict node affinity rule for vm:101 on node1:3,node2:2
- non-strict node affinity rule for vm:103 on node3:3,node2:2
This rule set would only be consistent if node1 and node3 would be both
down and would never fallback to node3 and node1 respectively as then
the whole cluster must be offline.
I'll clarify this in the commit message and test cases for the v3.
FWIW it might be worth to check out to loosen up this behavior a bit
with counting priorities as weights, as we briefly talked off-list as
far as I can remember, but as I've seen at least a few users who depend
on the current behavior we'd need to make this a (per-rule?) flag.
>>
>> Therefore build the same disjoint positive resource affinity resource
>> sets as the merge_connected_positive_resource_affinity_rules(...)
>> subroutine, so that the inconsistency check has the necessary
>> information in advance.
>>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
next prev parent reply other threads:[~2025-10-31 10:01 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-09 8:33 [pve-devel] [PATCH ha-manager v2 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 01/18] config: do not add ignored resources to dependent resources Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 02/18] manager: retranslate rules if nodes are added or removed Daniel Kral
2025-09-11 15:01 ` Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 03/18] rules: factor out disjoint rules' resource set helper Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 04/18] rules: resource affinity: inter-consistency check with merged positive rules Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 05/18] rules: add merged positive resource affinity info in global checks Daniel Kral
2025-09-10 17:35 ` Thomas Lamprecht
2025-10-31 10:01 ` Daniel Kral [this message]
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 06/18] rules: make rules sorting optional in foreach_rule helper Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 07/18] rename rule's canonicalize stage to transform stage Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 08/18] rules: make plugins register transformers instead of plugin_transform Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 09/18] rules: node affinity: decouple get_node_affinity helper from Usage class Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 10/18] compile ha rules to a more compact representation Daniel Kral
2025-10-13 7:50 ` Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 11/18] test: rules: use to_json instead of Data::Dumper for config output Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 12/18] test: rules: add compiled config output to rules config test cases Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 13/18] rules: node affinity: define node priority outside hash access Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 14/18] move minimum version check helper to ha tools Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 15/18] manager: move group migration cooldown variable into helper Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 16/18] api: status: sync active service counting with lrm's helper Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 17/18] manager: group migration: " Daniel Kral
2025-09-09 8:33 ` [pve-devel] [PATCH ha-manager v2 18/18] factor out counting of active services into helper 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=DDWF5LB1YXU2.1AQWDK136FJFE@proxmox.com \
--to=d.kral@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=t.lamprecht@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.