From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 6B10C1FF186 for ; Fri, 29 Aug 2025 14:43:51 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D93042E402; Fri, 29 Aug 2025 14:44:01 +0200 (CEST) Date: Fri, 29 Aug 2025 14:43:58 +0200 Message-Id: Cc: "pve-devel" From: =?utf-8?q?Michael_K=C3=B6ppl?= To: "Proxmox VE development discussion" Mime-Version: 1.0 X-Mailer: aerc 0.20.1 References: <20250821143705.256562-1-d.kral@proxmox.com> <20250821143705.256562-11-d.kral@proxmox.com> In-Reply-To: <20250821143705.256562-11-d.kral@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1756471430828 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.022 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH ha-manager 10/18] compile ha rules to a more compact representation X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" 2 comments inline On Thu Aug 21, 2025 at 4:35 PM CEST, Daniel Kral wrote: > sub write_rules_config { > @@ -391,8 +391,9 @@ sub get_resource_motion_info { > my $ss = $manager_status->{service_status}; > my $ns = $manager_status->{node_status}; > > - my $rules = read_and_check_effective_rules_config(); > - my ($together, $separate) = get_affinitive_resources($rules, $sid); > + my $affinity = read_and_compile_rules_config(); nit: maybe something like $compiled_rules? Calling a variable for this structure $affinity didn't make it quite clear to me what this is, especially when later on it's used as $affinity->{'resource-affinity'}. Maybe I'm just misinterpreting this, though. Or could you maybe explain why this compiled rule structure is then called affinity? > + my ($together, $separate) = > + get_affinitive_resources($affinity->{'resource-affinity'}, $sid); > > for my $csid (sort keys %$together) { > next if !defined($ss->{$csid}); > diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm > index 3013d369..748667e8 100644 > --- a/src/PVE/HA/Manager.pm > +++ b/src/PVE/HA/Manager.pm > @@ -121,11 +121,11 @@ sub flush_master_status { > > =head3 select_service_node(...) > > -=head3 select_service_node($rules, $online_node_usage, $sid, $service_conf, $sd, $node_preference) > +=head3 select_service_node($affinity, $online_node_usage, $sid, $service_conf, $sd, $node_preference) > > Used to select the best fitting node for the service C<$sid>, with the > -configuration C<$service_conf> and state C<$sd>, according to the rules defined > -in C<$rules>, available node utilization in C<$online_node_usage>, and the > +configuration C<$service_conf> and state C<$sd>, according to the affinity > +in C<$affinity>, available node utilization in C<$online_node_usage>, and the > given C<$node_preference>. > > The C<$node_preference> can be set to: > @@ -143,20 +143,21 @@ The C<$node_preference> can be set to: > =cut > > sub select_service_node { > - my ($rules, $online_node_usage, $sid, $service_conf, $sd, $node_preference) = @_; > + my ($affinity, $online_node_usage, $sid, $service_conf, $sd, $node_preference) = @_; > > die "'$node_preference' is not a valid node_preference for select_service_node\n" > if $node_preference !~ m/(none|best-score|try-next)/; > > my ($current_node, $tried_nodes, $maintenance_fallback) = > $sd->@{qw(node failed_nodes maintenance_node)}; > + my ($node_affinity, $resource_affinity) = $affinity->@{qw(node-affinity resource-affinity)}; > > my $online_nodes = { map { $_ => 1 } $online_node_usage->list_nodes() }; > - my ($allowed_nodes, $pri_nodes) = get_node_affinity($rules, $sid, $online_nodes); > + my ($allowed_nodes, $pri_nodes) = get_node_affinity($node_affinity, $sid, $online_nodes); > > return undef if !%$pri_nodes; > > - my ($together, $separate) = get_resource_affinity($rules, $sid, $online_node_usage); > + my ($together, $separate) = get_resource_affinity($resource_affinity, $sid, $online_node_usage); > > # stay on current node if possible (avoids random migrations) > if ( > @@ -418,7 +419,8 @@ sub execute_migration { > > my ($haenv, $ss) = $self->@{qw(haenv ss)}; > > - my ($together, $separate) = get_affinitive_resources($self->{rules}, $sid); > + my $resource_affinity = $self->{affinity}->{'resource-affinity'}; > + my ($together, $separate) = get_affinitive_resources($resource_affinity, $sid); > > for my $csid (sort keys %$separate) { > next if !defined($ss->{$csid}); > @@ -746,7 +748,7 @@ sub manage { > PVE::HA::Groups::migrate_groups_to_resources($self->{groups}, $sc); > > if ( > - !$self->{rules} > + !$self->{affinity} > || $has_changed_nodelist > || $new_rules->{digest} ne $self->{last_rules_digest} > || $self->{groups}->{digest} ne $self->{last_groups_digest} > @@ -756,11 +758,12 @@ sub manage { > > my $nodes = $self->{ns}->list_nodes(); > my $messages = PVE::HA::Rules->transform($new_rules, $nodes); > + my $new_affinity = PVE::HA::Rules->compile($new_rules, $nodes); > $haenv->log('info', $_) for @$messages; > > - $self->{rules} = $new_rules; > + $self->{affinity} = $new_affinity; > > - $self->{last_rules_digest} = $self->{rules}->{digest}; > + $self->{last_rules_digest} = $new_rules->{digest}; > $self->{last_groups_digest} = $self->{groups}->{digest}; > $self->{last_services_digest} = $services_digest; > } > @@ -1020,7 +1023,7 @@ sub next_state_request_start { > > if ($self->{crs}->{rebalance_on_request_start}) { > my $selected_node = select_service_node( > - $self->{rules}, > + $self->{affinity}, > $self->{online_node_usage}, > $sid, > $cd, > @@ -1187,7 +1190,7 @@ sub next_state_started { > } > > my $node = select_service_node( > - $self->{rules}, > + $self->{affinity}, > $self->{online_node_usage}, > $sid, > $cd, > @@ -1306,7 +1309,7 @@ sub next_state_recovery { > $self->recompute_online_node_usage(); # we want the most current node state > > my $recovery_node = select_service_node( > - $self->{rules}, > + $self->{affinity}, > $self->{online_node_usage}, > $sid, > $cd, > diff --git a/src/PVE/HA/Rules.pm b/src/PVE/HA/Rules.pm > index a075feac..d7593532 100644 > --- a/src/PVE/HA/Rules.pm > +++ b/src/PVE/HA/Rules.pm > @@ -47,6 +47,12 @@ Each I is required to implement the methods C>, > C>, and C> from the C> to > extend the properties of this I with plugin-specific properties. > > +Each I is required to implement the method > +Cplugin_compile(...) >>> to distill a compiled > +representation of the more verbose C<$rules> from the config file, which is > +returned by Ccompile(...) >>> to be more appropriate > +and efficient for the scheduler and other users. Is this supposed to say "other uses" or do you actually mean "other users"? Do you mean that this more efficient representation can (and probably should) be used for other uses cases when e.g. iterating over only resource affinity rules? _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel