Re: [PATCH perl-rs v2 16/40] pve-rs: resource-scheduling: expose auto rebalancing methods

["Dominik Rusovac" <d.rusovac@proxmox.com>]

lgtm

On Tue Mar 24, 2026 at 7:30 PM CET, Daniel Kral wrote:
> These methods expose the auto rebalancing methods of both the static and
> dynamic scheduler.
>
> As Scheduler::score_best_balancing_migration_candidates{,_topsis}()
> takes a possible very large list of migration candidates, the binding
> takes a more compact representation, which reduces the size that needs
> to be generated on the caller's side and therefore the runtime of the
> serialization from Perl to Rust.
>
> Additionally, while decomposing the compact representation the input
> data is validated since the underlying scoring methods do not further
> validate whether their input is consistent with the cluster usage.
>
> The method names score_best_balancing_migration_candidates{,_topsis}()
> are chosen deliberately, so that future extensions can implement
> score_best_balancing_migrations{,_topsis}(), which might allow to score
> migrations without providing the candidates.
>
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
> changes v1 -> v2:
> - improve patch message and documentation
> - move to the end of the perl-rs changes, which makes it more consistent
>   with the change order in pve-ha-manager as well
> - uses `UsageAggregator` now to discern how usages are accumulated
> - s/generate_migration_candidates_from
>    /decompose_compact_migration_candidates
> - make the decomposition of compact migration candidates more robust and
>   do not use any unwraps or other causes of panic but the Mutex guard
>   unwrap
>

[snip]

> +/// Transforms a `Vec<CompactMigrationCandidate>` to a `Vec<MigrationCandidate>` with the cluster
> +/// usage from `usage`.
> +///
> +/// This function fails for any of the following conditions for a [`CompactMigrationCandidate`]:
> +///
> +/// - the `leader` is not present in the cluster usage
> +/// - the `leader` is non-stationary
> +/// - any resource in `resources` is not present in the cluster usage
> +/// - any resource in `resources` is non-stationary
> +/// - any resource in `resources` is on another node than the `leader`

nice idea, to use Vec::with_capacity in here

> +pub(crate) fn decompose_compact_migration_candidates(
> +    usage: &Usage,
> +    compact_candidates: Vec<CompactMigrationCandidate>,
> +) -> Result<Vec<MigrationCandidate>, Error> {
> +    // The length of `compact_candidates` is at least a lower bound
> +    let mut candidates = Vec::with_capacity(compact_candidates.len());

[snip]

Reviewed-by: Dominik Rusovac <d.rusovac@proxmox.com>