public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Daniel Kral" <d.kral@proxmox.com>
To: "Proxmox VE development discussion" <pve-devel@lists.proxmox.com>
Cc: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH ha-manager 04/18] rules: resource affinity: inter-consistency check with merged positive rules
Date: Fri, 29 Aug 2025 15:28:16 +0200	[thread overview]
Message-ID: <DCEY3O6LBV4V.1K6P1E096PMRK@proxmox.com> (raw)
In-Reply-To: <DCEX53IDXEU5.3DIJ9S0X31DLU@proxmox.com>

On Fri Aug 29, 2025 at 2:43 PM CEST, Michael Köppl wrote:
> One nit inline
>
> On Thu Aug 21, 2025 at 4:35 PM CEST, Daniel Kral wrote:
>> @@ -248,58 +255,6 @@ __PACKAGE__->register_check(
>>  
>>  =cut
>>  
>> -my $sort_by_lowest_resource_id = sub {
>> -    my ($rules) = @_;
>> -
>> -    my $lowest_rule_resource_id = {};
>> -    for my $ruleid (keys %$rules) {
>> -        my @rule_resources = sort keys $rules->{$ruleid}->{resources}->%*;
>> -        $lowest_rule_resource_id->{$ruleid} = $rule_resources[0];
>> -    }
>> -
>> -    # sort rules such that rules with the lowest numbered resource come first
>> -    my @sorted_ruleids = sort {
>> -        $lowest_rule_resource_id->{$a} cmp $lowest_rule_resource_id->{$b}
>> -    } sort keys %$rules;
>> -
>> -    return @sorted_ruleids;
>> -};
>> -
>> -# returns a list of hashes, which contain disjoint resource affinity rules, i.e.,
>> -# put resource affinity constraints on disjoint sets of resources
>> -my $find_disjoint_resource_affinity_rules = sub {
>> -    my ($rules) = @_;
>> -
>> -    my @disjoint_rules = ();
>> -
>> -    # order needed so that it is easier to check whether there is an overlap
>> -    my @sorted_ruleids = $sort_by_lowest_resource_id->($rules);
>> -
>> -    for my $ruleid (@sorted_ruleids) {
>> -        my $rule = $rules->{$ruleid};
>> -
>> -        my $found = 0;
>> -        for my $entry (@disjoint_rules) {
>> -            next if sets_are_disjoint($rule->{resources}, $entry->{resources});
>> -
>> -            $found = 1;
>> -            push @{ $entry->{ruleids} }, $ruleid;
>> -            $entry->{resources}->{$_} = 1 for keys $rule->{resources}->%*;
>> -
>> -            last;
>> -        }
>> -        if (!$found) {
>> -            push @disjoint_rules,
>> -                {
>> -                    ruleids => [$ruleid],
>> -                    resources => { $rule->{resources}->%* },
>> -                };
>> -        }
>> -    }
>> -
>> -    return @disjoint_rules;
>> -};
>> -
>
> nit: I think the removal of this would have been better to do in the
> previous patch. It's not really related to the changes made in this
> patch AFAICT and the function is not used anymore at this point.

Ah right, ACK, I forgot to reply to this patch afterwards but I wrongly
squashed that in a rebase, it should have gone to the previous patch,
but thanks for pointing it out either way! I'll move it in a v2.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

  reply	other threads:[~2025-08-29 13:28 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-21 14:35 [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 01/18] config: do not add ignored resources to dependent resources Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 02/18] manager: retranslate rules if nodes are added or removed Daniel Kral
2025-08-27 13:41   ` Michael Köppl
2025-08-27 14:33     ` Daniel Kral
2025-08-27 14:55   ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 03/18] rules: factor out disjoint rules' resource set helper Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 04/18] rules: resource affinity: inter-consistency check with merged positive rules Daniel Kral
2025-08-29 12:43   ` Michael Köppl
2025-08-29 13:28     ` Daniel Kral [this message]
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 05/18] rules: add merged positive resource affinity info in global checks Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 06/18] rules: make rules sorting optional in foreach_rule helper Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 07/18] rename rule's canonicalize stage to transform stage Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 08/18] rules: make plugins register transformers instead of plugin_transform Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 09/18] rules: node affinity: decouple get_node_affinity helper from Usage class Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 10/18] compile ha rules to a more compact representation Daniel Kral
2025-08-27 15:19   ` Daniel Kral
2025-08-29 12:43   ` Michael Köppl
2025-08-29 13:42     ` Daniel Kral
2025-09-02  7:32       ` Michael Köppl
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 11/18] test: rules: use to_json instead of Data::Dumper for config output Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 12/18] test: rules: add compiled config output to rules config test cases Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 13/18] rules: node affinity: define node priority outside hash access Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 14/18] move minimum version check helper to ha tools Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 15/18] manager: move group migration cooldown variable into helper Daniel Kral
2025-08-29 12:43   ` Michael Köppl
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 16/18] api: status: sync active service counting with lrm's helper Daniel Kral
2025-09-02  8:10   ` Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 17/18] manager: group migration: " Daniel Kral
2025-08-21 14:35 ` [pve-devel] [PATCH ha-manager 18/18] factor out counting of active services into helper Daniel Kral
2025-08-29 12:44   ` Michael Köppl
2025-08-29 13:36     ` Daniel Kral
2025-08-29 12:44 ` [pve-devel] [PATCH ha-manager 00/18] HA rules fixes + cleanup + performance improvements Michael Köppl
2025-08-29 13:52   ` 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=DCEY3O6LBV4V.1K6P1E096PMRK@proxmox.com \
    --to=d.kral@proxmox.com \
    --cc=pve-devel-bounces@lists.proxmox.com \
    --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