Re: [PATCH ha-manager v3 35/40] implement automatic rebalancing

Dominik Rusovac
2026-03-31T09:38:46Z

On Tue Mar 31, 2026 at 11:32 AM CEST, Daniel Kral wrote:
> On Tue Mar 31, 2026 at 11:16 AM CEST, Dominik Rusovac wrote:
>> On Tue Mar 31, 2026 at 11:07 AM CEST, Michael Köppl wrote:
>>> 2 comments inline
>>>
>>> On Mon Mar 30, 2026 at 4:30 PM CEST, Daniel Kral wrote:
>>>
>>> [snip]
>>>
>>>> + my $candidates = $self->get_resource_migration_candidates();
>>>> +
>>>> + my $result;
>>>> + if ($method eq 'bruteforce') {
>>>> + $result = $online_node_usage->select_best_balancing_migration($candidates);
>>>> + } elsif ($method eq 'topsis') {
>>>> + $result = $online_node_usage->select_best_balancing_migration_topsis($candidates);
>>>> + }
>>>> +
>>>> + # happens if $candidates is empty or $method isn't handled above
>>>> + return if !$result;
>>>> +
>>>> + my ($migration, $target_imbalance) = $result->@{qw(migration imbalance)};
>>>> +
>>>> + my $relative_change = ($imbalance - $target_imbalance) / $imbalance;
>>>
>>> Since you get $imbalance from a function that returns 0.0 for the case
>>> that the cluster load is perfectly balanced (?), you could run into
>>> division by 0 here, no?
>>>
>>
>> technically this could happen, however an imbalance of 0.0 certainly
>> should not exceed a threshold (this is the case that "the cluster load
>> is perfectly balanced"); so the $relative_change ought to be never computed
>>
>
> Good catch, thanks to you both!
>
> Even though it's unpractical, users can still set the threshold to 0.0,
> which could actually cause a division by zero here, because the
> threshold is compared by a >= relation.
>
>
>
> # cat imbalance-zero.pl
> #!/usr/bin/perl
>
> use v5.36;
>
> my $imbalance = 0.0;
> my $threshold = 0.0;
> my $hold_duration = 3;
> my $sustained_imbalance_round = 0;
>
> sub test {
> if ($imbalance < $threshold) {
> $sustained_imbalance_round = 0;
> return;
> } else {
> $sustained_imbalance_round++;
> print "imbalance threshold exceeded\n";
> return if $sustained_imbalance_round < $hold_duration;
> print "sustained high imbalance\n";
> $sustained_imbalance_round = 0;
> }
>
> my $target_imbalance = 0.0;
> my $relative_change = ($imbalance - $target_imbalance) / $imbalance;
> }
>
> test();
> test();
> test();
> # chmod +x imbalance-zero.pl
> # ./imbalance-zero.pl
> imbalance threshold exceeded
> imbalance threshold exceeded
> imbalance threshold exceeded
> sustained high imbalance
> Illegal division by zero at ./imbalance-zero.pl line 23.
>
>
>
> The system is rather unstable in that regard anyway (same if $margin =
> 0.0), because it always tries to load balance every $hold_duration HA
> rounds.
>
> I'm not sure whether we should prevent this with adjusting the range for
> both the threshold and margin to be at least larger than some minimum
> value, so that the load balancing system won't become unstable.
>

yeah, either this, or you add a guard to return early (before the
threshold guard) whenever imbalance is 0.0, I guess

>>>> + return if $relative_change < $margin;
>>>> +
>>>> + my ($sid, $source, $target) = $migration->@{qw(sid source-node target-node)};
>>>> +
>>>> + my (undef, $type, $id) = $haenv->parse_sid($sid);
>>>> + my $task = $type eq 'vm' ? "migrate" : "relocate";
>>>> + my $cmd = "$task $sid $target";
>>>> +
>>>> + my $target_imbalance_str = int(100 * $target_imbalance + 0.5) / 100;
>>>> + $haenv->log(
>>>> + 'info',
>>>> + "auto rebalance - $task $sid to $target (expected target imbalance: $target_imbalance_str)",
>>>> + );
>>>> +
>>>> + $self->queue_resource_motion($cmd, $task, $sid, $target);
>>>> +}
>>>> +
>>
>> [snip]