[PATCH cluster/docs/manager v3 0/8] add warning messages for high token timeouts in clusters

[PATCH cluster/docs/manager v3 0/8] add warning messages for high token timeouts in clusters

[Michael Köppl]

This patch series introduces warnings informing users about high token
timeouts in their clusters. A recent change [0] lowered the token
coefficient for clusters and allowed adapting it. However, this change
only affects new clusters. As described in [1], users with existing
cluster should be informed about the high token timeouts in their
configurations and what they can do to alleviate this problem.

Thus, warnings are added to the `pvecm status` command as well as to the
cluster join info dialog in the web UI. The warning in the web UI warns
users about the effect adding another node would have to allow them to
make an informed change before adding another node.

changes since v2 (thanks to @Lukas and @Friedrich for the feedback):
- use "lowering" instead of "changing" for the warning strings
- replace leftover hard-coded PVE docs link with `man pvecm`
- update descriptions of timeout_warning_level and expected_timeout
  params such that they say "membership recovery timeout", to more
  clearly state which timeout this is about
- update function names for get_timeout_warning_level and
  get_timeout_warning to
  calculate_membership_recovery_timeout_warning_level and
  calculate_membership_recovery_timeout_warning respectively

changes since v1 (thanks to @Friedrich for the feedback on v1):
- add pve-docs patch to allow using get_help_link to directly link to
  an anchor in the local documentation (then used to link to the section
  on changing the token coefficient)
- add pve-docs patch to define explicit anchor for the "Changing the
  Token Coefficient" section
- add pve-docs patch extending the section for Changing the Token
  Coefficient slightly, informing users of the potential warning
  messages in the pvecm status output
- change the threshold for "strongly recommend" from 50s to 45s as
  suggested by @Friedrich
- adapted the name of calculate_total_timeout to
  calculate_membership_recovery_timeout
- adapted commit messages for preparatory pve-manager patch (no
  functional changes intended)
- moved the warning message in pvecm from `pvecm nodes` to `pvecm status`
- replaced the URL in the pvecm warning message with a reference to the
  pvecm man pages
- link to local documentation in ClusterEdit.js


pve-docs:

Michael Köppl (3):
  asciidoc-pve: allow linking sections with get_help_link
  pvecm: add explicit anchor for token coefficient section
  pvecm: add info about warnings regarding token coefficient

 pvecm.adoc              | 6 ++++++
 scripts/asciidoc-pve.in | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)


pve-cluster:

Michael Köppl (3):
  add functions to determine warning level for high token timeouts
  pvecm: warn users of high token timeouts when using status command
  api: add token timeout and warning level to cluster join info

 src/PVE/API2/ClusterConfig.pm | 24 +++++++++++++++++
 src/PVE/CLI/pvecm.pm          | 12 +++++++++
 src/PVE/Corosync.pm           | 50 +++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+)


pve-manager:

Michael Köppl (2):
  ui: cluster info: move initialization of items to initComponent
  ui: cluster info: warn users of high token timeout in join info

 www/manager6/dc/Cluster.js     |   4 +
 www/manager6/dc/ClusterEdit.js | 141 ++++++++++++++++++++++-----------
 2 files changed, 97 insertions(+), 48 deletions(-)


Summary over all repositories:
  7 files changed, 190 insertions(+), 49 deletions(-)

-- 
Generated by murpp 0.11.0

[PATCH docs v3 1/8] asciidoc-pve: allow linking sections with get_help_link

[Michael Köppl]

scan_extjs_file only scanned for occurrences of onlineHelp. Linking to
specific sections of the documentation with get_help_link was not
possible withouth an accompanying onlineHelp entry. Therefore, also scan
for occurrences of get_help_link.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
I'm sure this can be done more elegantly, but I wanted to avoid breaking
the existing regex for onlineHelp. Perhaps someone more knowledgeable in
regexes can point me in the right direction.

 scripts/asciidoc-pve.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/asciidoc-pve.in b/scripts/asciidoc-pve.in
index d42ddbe9..c4e72635 100644
--- a/scripts/asciidoc-pve.in
+++ b/scripts/asciidoc-pve.in
@@ -464,7 +464,7 @@ sub scan_extjs_file {
     debug("scan-extjs $filename");
 
     while (defined(my $line = <$fh>)) {
-        if ($line =~ m/\s+onlineHelp:\s*[\'\"]([^{}\[\]\'\"]+)[\'\"]/) {
+        if ($line =~ m/(?|\s+onlineHelp:\s*[\'\"]([^{}\[\]\'\"]+)[\'\"]|\bget_help_link\(\s*[\'\"]([^{}\[\]\'\"]+)[\'\"]\s*\))/) {
             my $blockid = $1;
             my $link = $fileinfo->{blockid_target}->{default}->{$blockid};
             if (!(defined($link) || defined($online_help_links->{$blockid}))) {
-- 
2.47.3

[PATCH docs v3 2/8] pvecm: add explicit anchor for token coefficient section

[Michael Köppl]

Suggested-by: Friedrich Weber <f.weber@proxmox.com>
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 pvecm.adoc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pvecm.adoc b/pvecm.adoc
index c09d19ff..2d14af11 100644
--- a/pvecm.adoc
+++ b/pvecm.adoc
@@ -1381,6 +1381,7 @@ systemctl restart corosync
 
 On errors, check the troubleshooting section below.
 
+[[pvecm_changing_token_coefficient]]
 Changing the Token Coefficient
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.47.3

[PATCH docs v3 3/8] pvecm: add info about warnings regarding token coefficient

[Michael Köppl]

`pvecm status` might warn users of a high sum of token and consensus
timeout, recommending lowering the token coefficient. To make users
aware that these warnings may occur and to allow users to search for
this warning in the docs and man pages, extend the section on lowering
the token coefficient.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 pvecm.adoc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/pvecm.adoc b/pvecm.adoc
index 2d14af11..3d652652 100644
--- a/pvecm.adoc
+++ b/pvecm.adoc
@@ -1399,6 +1399,11 @@ You can change the token coefficient of an existing cluster by
 xref:pvecm_edit_corosync_conf[editing corosync.conf]. Corosync will then
 automatically adopt the new value for the cluster.
 
+Cluster commands may display a warning if the sum of the Corosync token and
+consensus timeouts is considered too high (e.g., "Changing the token coefficient
+is recommended"). To resolve this warning, it is recommended to lower the token
+coefficient.
+
 Troubleshooting
 ~~~~~~~~~~~~~~~
 
-- 
2.47.3

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

[Michael Köppl]

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;
+}
+
+sub get_membership_recovery_timeout_warning_level {
+    my ($total_timeout_secs) = @_;
+
+    if ($total_timeout_secs > 45) {
+        return 'change-strongly-recommended';
+    } elsif ($total_timeout_secs > 40) {
+        return 'change-recommended';
+    } elsif ($total_timeout_secs > 30) {
+        return 'optimize';
+    }
+
+    return undef;
+}
+
+sub get_membership_recovery_timeout_warning {
+    my ($total_timeout_secs) = @_;
+
+    my $level = get_membership_recovery_timeout_warning_level($total_timeout_secs);
+    return undef if !defined($level);
+
+    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.";
+}
+
 1;
-- 
2.47.3

[PATCH cluster v3 5/8] pvecm: warn users of high token timeouts when using status command

[Michael Köppl]

If the calculated token timeout is above certain thresholds, display a
warning for users when running `pvecm status` as part of the Cluster
Information block. Also points users to the documentation regarding
potential adaptations to their cluster configuration to alleviate the
problem.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/CLI/pvecm.pm | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/PVE/CLI/pvecm.pm b/src/PVE/CLI/pvecm.pm
index 7d393a8..64e1d94 100755
--- a/src/PVE/CLI/pvecm.pm
+++ b/src/PVE/CLI/pvecm.pm
@@ -561,6 +561,18 @@ __PACKAGE__->register_method({
             $print_info->('Transport', 'transport', 'knet');
             $print_info->('Secure auth', 'secauth', 'off');
             printf "\n";
+
+            my $nodelist = PVE::Corosync::nodelist($conf);
+            my $total_timeout_secs = PVE::Corosync::calculate_membership_recovery_timeout(
+                $totem,
+                scalar(keys %$nodelist),
+            );
+            if (
+                my $msg =
+                PVE::Corosync::get_membership_recovery_timeout_warning($total_timeout_secs)
+            ) {
+                warn "$msg\n\n";
+            }
         }
 
         exec('corosync-quorumtool', '-siH');
-- 
2.47.3

[PATCH cluster v3 6/8] api: add token timeout and warning level to cluster join info

[Michael Köppl]

The token timeout in seconds and the warning level provide additional
information for users regarding the expected token timeout in seconds
after adding an additional node and whether changing the token
coefficient is recommended.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/API2/ClusterConfig.pm | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/src/PVE/API2/ClusterConfig.pm b/src/PVE/API2/ClusterConfig.pm
index bbed40e..e3a07cd 100644
--- a/src/PVE/API2/ClusterConfig.pm
+++ b/src/PVE/API2/ClusterConfig.pm
@@ -571,6 +571,19 @@ __PACKAGE__->register_method({
             preferred_node => get_standard_option('pve-node'),
             totem => { type => 'object' },
             config_digest => { type => 'string' },
+            expected_timeout => {
+                type => 'number',
+                description =>
+                    "Expected total membership recovery timeout (in seconds) if an additional node is added.",
+                optional => 1,
+            },
+            timeout_warning_level => {
+                type => 'string',
+                description =>
+                    "Warning level for the expected total membership recovery timeout.",
+                optional => 1,
+                enum => ['optimize', 'change-recommended', 'change-strongly-recommended'],
+            },
         },
     },
     code => sub {
@@ -599,12 +612,23 @@ __PACKAGE__->register_method({
             $node->{pve_addr} = scalar(PVE::Cluster::remote_node_ip($name));
         }
 
+        # Total timeout if additional node is added
+        my $total_timeout_secs = PVE::Corosync::calculate_membership_recovery_timeout(
+            $totem_cfg,
+            scalar(keys %$nodelist) + 1,
+        );
+
+        my $warning_level =
+            PVE::Corosync::get_membership_recovery_timeout_warning_level($total_timeout_secs);
+
         my $res = {
             nodelist => [values %$nodelist],
             preferred_node => $nodename,
             totem => $totem_cfg,
             config_digest => $corosync_config_digest,
+            expected_timeout => $total_timeout_secs,
         };
+        $res->{timeout_warning_level} = $warning_level if defined($warning_level);
 
         return $res;
     },
-- 
2.47.3

[PATCH manager v3 7/8] ui: cluster info: move initialization of items to initComponent

[Michael Köppl]

This allows conditionally adding items to the form. No functional
changes intended.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 www/manager6/dc/ClusterEdit.js | 102 +++++++++++++++++----------------
 1 file changed, 54 insertions(+), 48 deletions(-)

diff --git a/www/manager6/dc/ClusterEdit.js b/www/manager6/dc/ClusterEdit.js
index 109325855..aff1515ab 100644
--- a/www/manager6/dc/ClusterEdit.js
+++ b/www/manager6/dc/ClusterEdit.js
@@ -57,58 +57,64 @@ Ext.define('PVE.ClusterInfoWindow', {
         totem: {},
     },
 
-    items: [
-        {
-            xtype: 'component',
-            border: false,
-            padding: '10 10 10 10',
-            html: gettext('Copy the Join Information here and use it on the node you want to add.'),
-        },
-        {
-            xtype: 'container',
-            layout: 'form',
-            border: false,
-            padding: '0 10 10 10',
-            items: [
-                {
-                    xtype: 'textfield',
-                    fieldLabel: gettext('IP Address'),
-                    cbind: {
-                        value: '{joinInfo.ipAddress}',
-                    },
-                    editable: false,
-                },
-                {
-                    xtype: 'textfield',
-                    fieldLabel: gettext('Fingerprint'),
-                    cbind: {
-                        value: '{joinInfo.fingerprint}',
+    initComponent: function () {
+        var me = this;
+
+        var joinInfo = me.joinInfo;
+
+        me.items = [];
+
+        me.items.push(
+            {
+                xtype: 'component',
+                border: false,
+                padding: '10 10 10 10',
+                html: gettext(
+                    'Copy the Join Information here and use it on the node you want to add.',
+                ),
+            },
+            {
+                xtype: 'container',
+                layout: 'form',
+                border: false,
+                padding: '0 10 10 10',
+                items: [
+                    {
+                        xtype: 'textfield',
+                        fieldLabel: gettext('IP Address'),
+                        value: joinInfo.ipAddress,
+                        editable: false,
                     },
-                    editable: false,
-                },
-                {
-                    xtype: 'textarea',
-                    inputId: 'pveSerializedClusterInfo',
-                    fieldLabel: gettext('Join Information'),
-                    grow: true,
-                    cbind: {
-                        joinInfo: '{joinInfo}',
+                    {
+                        xtype: 'textfield',
+                        fieldLabel: gettext('Fingerprint'),
+                        value: joinInfo.fingerprint,
+                        editable: false,
                     },
-                    editable: false,
-                    listeners: {
-                        afterrender: function (field) {
-                            if (!field.joinInfo) {
-                                return;
-                            }
-                            var jsons = Ext.JSON.encode(field.joinInfo);
-                            var base64s = Ext.util.Base64.encode(jsons);
-                            field.setValue(base64s);
+                    {
+                        xtype: 'textarea',
+                        inputId: 'pveSerializedClusterInfo',
+                        fieldLabel: gettext('Join Information'),
+                        grow: true,
+                        joinInfo: joinInfo,
+                        editable: false,
+                        listeners: {
+                            afterrender: function (field) {
+                                if (!field.joinInfo) {
+                                    return;
+                                }
+                                var jsons = Ext.JSON.encode(field.joinInfo);
+                                var base64s = Ext.util.Base64.encode(jsons);
+                                field.setValue(base64s);
+                            },
                         },
                     },
-                },
-            ],
-        },
-    ],
+                ],
+            },
+        );
+
+        me.callParent();
+    },
     dockedItems: [
         {
             dock: 'bottom',
-- 
2.47.3

[PATCH manager v3 8/8] ui: cluster info: warn users of high token timeout in join info

[Michael Köppl]

If another node would increase Corosync's token timeout to a level that
might affect the stability of the cluster, display a warning hint to
users, pointing them to the documentation section about changing the
token coefficient, allowing them to make an informed change before
another node.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 www/manager6/dc/Cluster.js     |  4 ++++
 www/manager6/dc/ClusterEdit.js | 39 ++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/www/manager6/dc/Cluster.js b/www/manager6/dc/Cluster.js
index 2ec5588c3..00138f328 100644
--- a/www/manager6/dc/Cluster.js
+++ b/www/manager6/dc/Cluster.js
@@ -91,6 +91,8 @@ Ext.define('PVE.ClusterAdministration', {
                     vm.set('totem', data.totem);
                     vm.set('isInCluster', !!data.totem.cluster_name);
                     vm.set('nodelist', data.nodelist);
+                    vm.set('expected_timeout', data.expected_timeout);
+                    vm.set('timeout_warning_level', data.timeout_warning_level);
 
                     let nodeinfo = data.nodelist.find((el) => el.name === data.preferred_node);
 
@@ -133,6 +135,8 @@ Ext.define('PVE.ClusterAdministration', {
                             peerLinks: vm.get('preferred_node.peerLinks'),
                             ring_addr: vm.get('preferred_node.ring_addr'),
                             totem: vm.get('totem'),
+                            expected_timeout: vm.get('expected_timeout'),
+                            timeout_warning_level: vm.get('timeout_warning_level'),
                         },
                     });
                 },
diff --git a/www/manager6/dc/ClusterEdit.js b/www/manager6/dc/ClusterEdit.js
index aff1515ab..800ae5dd4 100644
--- a/www/manager6/dc/ClusterEdit.js
+++ b/www/manager6/dc/ClusterEdit.js
@@ -55,6 +55,8 @@ Ext.define('PVE.ClusterInfoWindow', {
         ipAddress: undefined,
         fingerprint: undefined,
         totem: {},
+        expected_timeout: undefined,
+        timeout_warning_level: undefined,
     },
 
     initComponent: function () {
@@ -113,6 +115,43 @@ Ext.define('PVE.ClusterInfoWindow', {
             },
         );
 
+        if (joinInfo.expected_timeout && joinInfo.timeout_warning_level) {
+            let level;
+            if (joinInfo.timeout_warning_level === 'change-strongly-recommended') {
+                level = gettext('Lowering the token coefficient is strongly recommended');
+            } else if (joinInfo.timeout_warning_level === 'change-recommended') {
+                level = gettext('Lowering the token coefficient is recommended');
+            } else if (joinInfo.timeout_warning_level === 'optimize') {
+                level = gettext('The token coefficient can be optimized');
+            }
+
+            let msg = Ext.String.format(
+                gettext(
+                    "Adding another node will increase the sum of Corosync's token and consensus timeout to {0}s. {1}." +
+                        ' See {2} for details.',
+                ),
+                joinInfo.expected_timeout,
+                level,
+                '<a target="_blank" href="' +
+                    Proxmox.Utils.get_help_link('pvecm_changing_token_coefficient') +
+                    '">the documentation</a>',
+            );
+
+            me.items.push({
+                xtype: 'container',
+                border: false,
+                padding: '0 10 10 10',
+                items: [
+                    {
+                        itemId: 'joinInfoWarningHint',
+                        xtype: 'displayfield',
+                        userCls: 'pmx-hint',
+                        value: msg,
+                    },
+                ],
+            });
+        }
+
         me.callParent();
     },
     dockedItems: [
-- 
2.47.3