bug#43773: [PATCH] offload: Improve load normalization and configurability.

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

Hello,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Fixes <https://issues.guix.gnu.org/43773>.
>>
>> The computed normalized load was previously obtained by dividing the load
>> average as found in /proc/loadavg by the number of parallel builds defined for
>> a build machine.
>>
>> This normalized didn't allow to compare machines with different number of
>                               ^
>
>> cores, as the load average reported by can be as high as the number of cores;
>                                         ^
> Missing words.

Good catch, fixed.

[...]

>>  (define (node-load node)
>> -  "Return the load on NODE.  Return + if NODE is misbehaving."
>> +  "Return the load on NODE, a normalized value between 0.0 and 1.0.  The value
>> +is derived from /proc/loadavg and normalized according to the number of
>> +logical cores available, to give a rough estimation of CPU usage.  Return
>> +1.0 (fully loaded) if NODE is misbehaving."
>>    (let ((line (inferior-eval '(begin
>>                                  (use-modules (ice-9 rdelim))
>>                                  (call-with-input-file "/proc/loadavg"
>>                                    read-string))
>> -                             node)))
>> -    (if (eof-object? line)
>> -        +inf.0 ;MACHINE does not respond, so assume it is infinitely loaded
>> +                             node))
>> +        (ncores (inferior-eval '(begin
>> +                                  (use-modules (ice-9 threads))
>> +                                  (current-processor-count))
>> +                               node)))
>> +    (if (or (eof-object? line) (eof-object? ncores))
>> +        1.0    ;MACHINE does not respond, so assume it is fully loaded
>
> Returning 1.0 now is akin to returning + before, meaning that the
> machine will never be picked up, is that right?

Yes, 1.0 has the same meaning as the +inf.0 value previously used (i.e.,
the machine is fully loaded).

> What if one sets overload-threshold = 1.0, the machine would still be
> picked up, no?

Currently no, the machine would never be picked up, as the maximum value
returned by node-load is now 1.0, and the comparison is using the
strictly inferior to operator (<).  Perhaps this should be made a <=
operator?

>> +         (if (and node
>> +                  (or (not threshold) (< load threshold))
>
> I think we can assume that THRESHOLD is always a number, including
> possible +inf.0.

It's no longer possible for node-load to return +inf.0; it's strictly
bound between 0.0 and 1.0.  The check for #f is done because it is
desirable (for semantic clarity) to allow the user to disable
overload-threshold altogether by setting it to #f.  This is documented.

Thanks!

Maxim