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

[PATCH cluster/manager 0/5] 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 nodes` 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.

[0] https://git.proxmox.com/?p=pve-cluster.git;a=commit;h=a7b1c765b9223a81fb2dc4f072d6a6c095583cda
[1] https://bugzilla.proxmox.com/show_bug.cgi?id=7398

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 nodes command
  api: add token timeout and warning level to cluster join info

 src/PVE/API2/ClusterConfig.pm | 20 ++++++++++++++
 src/PVE/CLI/pvecm.pm          |  9 +++++++
 src/PVE/Corosync.pm           | 50 +++++++++++++++++++++++++++++++++++
 3 files changed, 79 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 | 139 +++++++++++++++++++++------------
 2 files changed, 95 insertions(+), 48 deletions(-)


Summary over all repositories:
  5 files changed, 174 insertions(+), 48 deletions(-)

-- 
Generated by murpp 0.11.0

[PATCH cluster 1/5] 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>
---
The timeouts are chosen according to Friedrich's description in [0].

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=7398

 src/PVE/Corosync.pm | 50 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/src/PVE/Corosync.pm b/src/PVE/Corosync.pm
index aef0d31..41d4c6f 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_total_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_timeout_warning_level {
+    my ($total_timeout_secs) = @_;
+
+    if ($total_timeout_secs > 50) {
+        return 'change-strongly-recommended';
+    } elsif ($total_timeout_secs > 40) {
+        return 'change-recommended';
+    } elsif ($total_timeout_secs > 30) {
+        return 'optimize';
+    }
+
+    return undef;
+}
+
+sub get_timeout_warning {
+    my ($total_timeout_secs) = @_;
+
+    my $level = get_timeout_warning_level($total_timeout_secs);
+    return undef if !defined($level);
+
+    my $level_msg;
+    if ($level eq 'change-strongly-recommended') {
+        $level_msg = "Changing the token coefficient is strongly recommended";
+    } elsif ($level eq 'change-recommended') {
+        $level_msg = "Changing the token coefficient is recommended";
+    } elsif ($level eq 'optimize') {
+        $level_msg = "Token coefficient can be optimized";
+    }
+
+    return
+        "Sum of Corosync token and consensus timeout is ${total_timeout_secs}s. "
+        . "$level_msg. "
+        . "See https://pve.proxmox.com/pve-docs/chapter-pvecm.html#_changing_the_token_coefficient for details.";
+}
+
 1;
-- 
2.47.3

[PATCH cluster 2/5] pvecm: warn users of high token timeouts when using nodes command

[Michael Köppl]

If the calculated token timeout is above certain thresholds, display a
warning for users when running `pvecm nodes`. 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 | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/PVE/CLI/pvecm.pm b/src/PVE/CLI/pvecm.pm
index 7d393a8..561c5f6 100755
--- a/src/PVE/CLI/pvecm.pm
+++ b/src/PVE/CLI/pvecm.pm
@@ -584,6 +584,15 @@ __PACKAGE__->register_method({
 
         PVE::Corosync::check_conf_exists();
 
+        my $conf = PVE::Cluster::cfs_read_file("corosync.conf");
+        my $nodelist = PVE::Corosync::nodelist($conf);
+        my $totem_cfg = PVE::Corosync::totem_config($conf);
+        my $total_timeout_secs =
+            PVE::Corosync::calculate_total_timeout($totem_cfg, scalar(keys %$nodelist));
+        if (my $msg = PVE::Corosync::get_timeout_warning($total_timeout_secs)) {
+            warn "$msg\n";
+        }
+
         exec('corosync-quorumtool', '-l');
         exit(-1); # should not be reached
     },
-- 
2.47.3

[PATCH cluster 3/5] 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>
---
I'm not quite sure regarding this one, so please feel free to provide
feedback here. I decided against simply returning the same warning
message as in pvecm here because I wanted to avoid returning a warning
message with a URL from the API. However, I still wanted to provide
information regarding the high token timeout to users who call the API
directly. These 2 fields give enough flexibility for the web UI while
still returning documented properties.

 src/PVE/API2/ClusterConfig.pm | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/src/PVE/API2/ClusterConfig.pm b/src/PVE/API2/ClusterConfig.pm
index bbed40e..6a0f99c 100644
--- a/src/PVE/API2/ClusterConfig.pm
+++ b/src/PVE/API2/ClusterConfig.pm
@@ -571,6 +571,18 @@ __PACKAGE__->register_method({
             preferred_node => get_standard_option('pve-node'),
             totem => { type => 'object' },
             config_digest => { type => 'string' },
+            expected_timeout => {
+                type => 'number',
+                description =>
+                    "Expected total timeout (in seconds) if an additional node is added.",
+                optional => 1,
+            },
+            timeout_warning_level => {
+                type => 'string',
+                description => "Warning level for the expected total timeout.",
+                optional => 1,
+                enum => ['optimize', 'change-recommended', 'change-strongly-recommended'],
+            },
         },
     },
     code => sub {
@@ -599,12 +611,20 @@ __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_total_timeout($totem_cfg, scalar(keys %$nodelist) + 1);
+
+        my $warning_level = PVE::Corosync::get_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 4/5] ui: cluster info: move initialization of items to initComponent

[Michael Köppl]

This allows conditionally adding items to the form.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
Preparatory patch, no functional changes intended.

 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 5/5] 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 | 37 ++++++++++++++++++++++++++++++++++
 2 files changed, 41 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..a1720dbca 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,41 @@ Ext.define('PVE.ClusterInfoWindow', {
             },
         );
 
+        if (joinInfo.expected_timeout && joinInfo.timeout_warning_level) {
+            let level;
+            if (joinInfo.timeout_warning_level === 'change-strongly-recommended') {
+                level = gettext('Changing token coefficient is strongly recommended');
+            } else if (joinInfo.timeout_warning_level === 'change-recommended') {
+                level = gettext('Changing token coefficient is recommended');
+            } else if (joinInfo.timeout_warning_level === 'optimize') {
+                level = gettext('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="https://pve.proxmox.com/pve-docs/chapter-pvecm.html#_changing_the_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