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 7C2DE1FF0C0 for ; Wed, 21 Oct 2026 14:50:06 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id DFC7D2159D; Wed, 21 Oct 2026 14:50:03 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Wed, 21 Oct 2026 14:49:59 +0200 Message-Id: To: "Dominik Rusovac" , Subject: Re: [PATCH proxmox 02/15] resource-scheduling: scheduler: abstract post-migration stats logic From: "Daniel Kral" X-Mailer: aerc 0.22.0-10-g6373ac9d2179-dirty References: <20261021085932.1196302-1-d.rusovac@proxmox.com> <20261021085932.1196302-3-d.rusovac@proxmox.com> In-Reply-To: <20261021085932.1196302-3-d.rusovac@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1792586999313 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.761 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium 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: ZTYWJ7PQM7SGUAL7INP3E6VNP72X3IFA X-Message-ID-Hash: ZTYWJ7PQM7SGUAL7INP3E6VNP72X3IFA 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: Nice! On Wed Oct 21, 2026 at 10:59 AM CEST, Dominik Rusovac wrote: > DRY and hide repeatedly used closure inside reuasble function. nit: s/reuasble/reusable/ > > Signed-off-by: Dominik Rusovac > --- > proxmox-resource-scheduling/src/scheduler.rs | 41 ++++++++------------ > 1 file changed, 16 insertions(+), 25 deletions(-) > > diff --git a/proxmox-resource-scheduling/src/scheduler.rs b/proxmox-resou= rce-scheduling/src/scheduler.rs > index 9078fa72..a12cd519 100644 > --- a/proxmox-resource-scheduling/src/scheduler.rs > +++ b/proxmox-resource-scheduling/src/scheduler.rs > @@ -56,6 +56,19 @@ fn calculate_node_imbalance(nodes: &[NodeUsage], to_lo= ad: impl Fn(&NodeUsage) -> > cv / max_cv > } > =20 > +/// Returns the hypothetical [`NodeStats`] of `node` with respect to the= migration `candidate`. > +fn node_stats_after_migration(candidate: &MigrationCandidate, node: &Nod= eUsage) -> NodeStats { > + let mut new_stats =3D node.stats; > + > + if node.name =3D=3D candidate.migration.source_node { > + new_stats.remove_running_resource(&candidate.stats); > + } else if node.name =3D=3D candidate.migration.target_node { > + new_stats.add_running_resource(&candidate.stats); > + } > + > + new_stats > +} > + > criteria_struct! { > /// A given alternative. > struct PveTopsisAlternative { > @@ -216,15 +229,7 @@ impl Scheduler { > /// See [`calculate_node_imbalance`] for more information. > fn node_imbalance_with_migration_candidate(&self, candidate: &Migrat= ionCandidate) -> f64 { > calculate_node_imbalance(&self.nodes, |node| { > - let mut new_stats =3D node.stats; > - > - if node.name =3D=3D candidate.migration.source_node { > - new_stats.remove_running_resource(&candidate.stats); > - } else if node.name =3D=3D candidate.migration.target_node { > - new_stats.add_running_resource(&candidate.stats); > - } > - > - new_stats.load() > + node_stats_after_migration(candidate, node).load() > }) nit: if it fits on a line, could drop the braces {} here > } > =20 > @@ -288,22 +293,8 @@ impl Scheduler { > let matrix =3D candidates > .iter() > .map(|candidate| { > - let resource_stats =3D &candidate.stats; > - let source_node =3D &candidate.migration.source_node; > - let target_node =3D &candidate.migration.target_node; > - > - self.topsis_alternative_with(|node| { > - let mut new_stats =3D node.stats; > - > - if &node.name =3D=3D source_node { > - new_stats.remove_running_resource(resource_stats= ); > - } else if &node.name =3D=3D target_node { > - new_stats.add_running_resource(resource_stats); > - } > - > - new_stats > - }) > - .into() > + self.topsis_alternative_with(|node| node_stats_after_mig= ration(candidate, node)) > + .into() > }) > .collect::>(); > =20 Otherwise, consider as: Reviewed-by: Daniel Kral