Re: [PATCH cluster v3 4/8] add functions to determine warning level for high token timeouts

["Michael Köppl" <m.koeppl@proxmox.com>]

On Mon May 18, 2026 at 4:11 PM CEST, Fabian Grünbichler wrote:
> On April 27, 2026 7:05 pm, Michael Köppl wrote:
>> High token timeouts can lead to stability problems in clusters. To
>> inform users about the timeout in their current setup (or expected
>> timeouts when adding nodes) and give recommendations regarding the token
>> coefficient setting, introduce function to calculate the timeout as well
>> as determine the warning / recommendation levels.
>> 
>> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
>> ---
>>  src/PVE/Corosync.pm | 50 +++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 50 insertions(+)
>> 
>> diff --git a/src/PVE/Corosync.pm b/src/PVE/Corosync.pm
>> index aef0d31..45a1f71 100644
>> --- a/src/PVE/Corosync.pm
>> +++ b/src/PVE/Corosync.pm
>> @@ -534,4 +534,54 @@ sub resolve_hostname_like_corosync {
>>      return $match_ip_and_version->($resolved_ip);
>>  }
>>  
>> +sub calculate_membership_recovery_timeout {
>> +    my ($totemcfg, $node_count) = @_;
>> +
>> +    my $token_timeout = $totemcfg->{token} // 3000;
>> +    my $token_coefficient = $totemcfg->{token_coefficient} // 650;
>> +
>> +    my $expected_token_timeout = $token_timeout;
>> +    if ($node_count > 2) {
>> +        $expected_token_timeout += ($node_count - 2) * $token_coefficient;
>> +    }
>> +
>> +    my $expected_consensus_timeout = $totemcfg->{consensus} // $expected_token_timeout * 1.2;
>> +    return ($expected_token_timeout + $expected_consensus_timeout) / 1000.0;
>
> we could also ask corosync (via corosync-cmapctl) about most of these,
> to avoid duplicating the calculations/defaults. the only thing missing
> is the coefficient, though we could probably expose that on the corosync
> side as well.

Thanks for having a look at this!

In the original implementation I used the values from cmap directly. The
reason I decided to implement it like this later on was that I wanted to
be able to calculate the timeout for an arbitrary number of nodes
(although n and n+1 would suffice) to be able to display a warning
before adding another node if the timeout would increase to a
"problematic" level. I suppose using the values from corosync-cmapctl
and then adding $node_delta * $token_coefficient to the token timeout
would work, but apart from the avoiding duplicating the defaults, I'm
not sure this would improve the solution much? Or am I missing
something here?

>
>> +}
>> +
>> +sub get_membership_recovery_timeout_warning_level {
>> +    my ($total_timeout_secs) = @_;
>> +

[snip]

>> +    my $level_msg;
>> +    if ($level eq 'change-strongly-recommended') {
>> +        $level_msg = "Lowering the token coefficient is strongly recommended";
>> +    } elsif ($level eq 'change-recommended') {
>> +        $level_msg = "Lowering the token coefficient is recommended";
>> +    } elsif ($level eq 'optimize') {
>> +        $level_msg = "The token coefficient can be optimized";
>> +    }
>> +
>> +    return
>> +        "Sum of Corosync token and consensus timeout is ${total_timeout_secs}s. "
>> +        . "$level_msg. "
>> +        . "See 'man pvecm' for details.";
>
> this pretty much duplicates the frontend code - if we leave out the last
> line we could just return the warning message, and call the field in the
> API return value "totem_warning(s)" or "health_warnings" or just
> "warnings" and potentially add more information in the future? we could
> still keep the level and return
>
> warnings = [ 
>  level => ...,
>  msg => ...,
> ]
>
> but I don't currently see a reason why we'd benefit from returning raw
> values and constructing the warning message on both ends?

The messages themselves differ because one warning message is for the
current state, whereas the other is for what would happen if another
node was added to the cluster, but I agree that it's unnecessarily
duplicated. We could instead return the warning message as
totem_warnings, as you suggested, but offer different warning messages
depending on a $node_delta (+ how many nodes to the current state, which
will pretty much be 1 for all cases right now)?

>
>> +}
>> +
>>  1;
>> -- 
>> 2.47.3
>> 
>> 
>> 
>> 
>> 
>>