[pve-devel] SPAM: [PATCH manager] ui: dc summary: fix calculation of shared storage size

[pve-devel] SPAM: [PATCH manager] ui: dc summary: fix calculation of shared storage size

[Igor Thaller via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 4721 bytes --]

From: Igor Thaller <igor.thaller@aon.at>
To: pve-devel@lists.proxmox.com
Cc: Igor Thaller <igor.thaller@aon.at>, Friedrich Weber <f.weber@proxmox.com>
Subject: SPAM: [PATCH manager] ui: dc summary: fix calculation of shared storage size
Date: Tue, 30 Jul 2024 15:44:32 +0200
Message-ID: <20240730134432.110382-1-igor.thaller@aon.at>

The issue is related to the 'Summary' tab under 'Datacenter' inside a
cluster. To get a steady reading of the storage size data, the frontend
requests the '/api2/json/cluster/resources' every three seconds to
retrieve the necessary data to calculate the used and total storage
size.

The problem occurs when a shared storage is defined and a node goes
offline. As the node is not online, it cannot report the shared storage
size (both used and total) back to the other nodes. The order of the
JSON response is not always the same, so it is possible that the offline
node will appear first. Consequently, the front-end will display the
wrong total and used storage. This is because the shared storage data
has both the maximum disk size and the used disk set to zero when the
node is offline. This causes the total and used space data to be
calculated and displayed incorrectly, leading to fluctuations in the
displayed percentage of used disk space.

To fix this, a conditional check to skip the storage report if its
status is 'unknown' and it is shared was added. This prevents the
unreliable data from being processed.

To test these changes, adjust the 'max_requests' variable in the Perl
script located at '/usr/share/perl5/PVE/Service/pveproxy.pm' to increase
the likelihood of the error to occur. This makes the storage size
fluctuations more frequent. Then compare the storage results (both used
and total sizes) before and after implementing the fix.

Note: Be aware that it takes around one minute for the spike to happen.

Reported-by: Friedrich Weber <f.weber@proxmox.com>
Signed-off-by: Igor Thaller <igor.thaller@aon.at>
---
 www/manager6/dc/Summary.js | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/www/manager6/dc/Summary.js b/www/manager6/dc/Summary.js
index efb44dae..9b8f3280 100644
--- a/www/manager6/dc/Summary.js
+++ b/www/manager6/dc/Summary.js
@@ -170,6 +170,11 @@ Ext.define('PVE.dc.Summary', {
 			} else if (countedStorage[sid]) {
 			    break;
 			}
+
+			if (data.status === "unknown" && data.shared) {
+			    break;
+			}
+
 			used += data.disk;
 			total += data.maxdisk;
 			countedStorage[sid] = true;
-- 
2.39.2



[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH manager] ui: dc summary: fix calculation of shared storage size

[Fiona Ebner]

Am 30.07.24 um 15:44 schrieb Igor Thaller via pve-devel:
> The issue is related to the 'Summary' tab under 'Datacenter' inside a
> cluster. To get a steady reading of the storage size data, the frontend
> requests the '/api2/json/cluster/resources' every three seconds to
> retrieve the necessary data to calculate the used and total storage
> size.
> 
> The problem occurs when a shared storage is defined and a node goes
> offline. As the node is not online, it cannot report the shared storage
> size (both used and total) back to the other nodes. The order of the
> JSON response is not always the same, so it is possible that the offline
> node will appear first. Consequently, the front-end will display the
> wrong total and used storage. This is because the shared storage data
> has both the maximum disk size and the used disk set to zero when the
> node is offline. This causes the total and used space data to be
> calculated and displayed incorrectly, leading to fluctuations in the
> displayed percentage of used disk space.
> 
> To fix this, a conditional check to skip the storage report if its
> status is 'unknown' and it is shared was added. This prevents the
> unreliable data from being processed.
> 

Nit: "and it is shared was added" could be improved language-wise

While the issue is seen with shared storages, would there be a downside
of skipping non-shared storages with status "unknown" too? From a quick
look, it seems that disk/maxdisk not being set does not depend on the
storage being shared, so I'd rather skip the addition with unset values too.

> To test these changes, adjust the 'max_requests' variable in the Perl
> script located at '/usr/share/perl5/PVE/Service/pveproxy.pm' to increase
> the likelihood of the error to occur. This makes the storage size
> fluctuations more frequent. Then compare the storage results (both used
> and total sizes) before and after implementing the fix.
> 
> Note: Be aware that it takes around one minute for the spike to happen.
> 

IMHO this part should not go into the commit message, but be mentioned
below the "---"

> Reported-by: Friedrich Weber <f.weber@proxmox.com>
> Signed-off-by: Igor Thaller <igor.thaller@aon.at>
> ---
>  www/manager6/dc/Summary.js | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/www/manager6/dc/Summary.js b/www/manager6/dc/Summary.js
> index efb44dae..9b8f3280 100644
> --- a/www/manager6/dc/Summary.js
> +++ b/www/manager6/dc/Summary.js
> @@ -170,6 +170,11 @@ Ext.define('PVE.dc.Summary', {
>  			} else if (countedStorage[sid]) {
>  			    break;
>  			}
> +
> +			if (data.status === "unknown" && data.shared) {
> +			    break;
> +			}
> +
>  			used += data.disk;
>  			total += data.maxdisk;
>  			countedStorage[sid] = true;
> -- 
> 2.39.2
> 
> 


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH manager] ui: dc summary: fix calculation of shared storage size

[Friedrich Weber]

Igor sent a v2:

https://lore.proxmox.com/pve-devel/mailman.73.1722428094.302.pve-devel@lists.proxmox.com/T/#u


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] SPAM: [PATCH manager] ui: dc summary: fix calculation of shared storage size

[Shannon Sterz]

On Tue Jul 30, 2024 at 3:44 PM CEST, Igor Thaller wrote:
> The issue is related to the 'Summary' tab under 'Datacenter' inside a
> cluster. To get a steady reading of the storage size data, the frontend
> requests the '/api2/json/cluster/resources' every three seconds to
> retrieve the necessary data to calculate the used and total storage
> size.
>
> The problem occurs when a shared storage is defined and a node goes
> offline. As the node is not online, it cannot report the shared storage
> size (both used and total) back to the other nodes. The order of the
> JSON response is not always the same, so it is possible that the offline
> node will appear first. Consequently, the front-end will display the
> wrong total and used storage. This is because the shared storage data
> has both the maximum disk size and the used disk set to zero when the
> node is offline. This causes the total and used space data to be
> calculated and displayed incorrectly, leading to fluctuations in the
> displayed percentage of used disk space.
>
> To fix this, a conditional check to skip the storage report if its
> status is 'unknown' and it is shared was added. This prevents the
> unreliable data from being processed.
>
> To test these changes, adjust the 'max_requests' variable in the Perl
> script located at '/usr/share/perl5/PVE/Service/pveproxy.pm' to increase
> the likelihood of the error to occur. This makes the storage size
> fluctuations more frequent. Then compare the storage results (both used
> and total sizes) before and after implementing the fix.
>
> Note: Be aware that it takes around one minute for the spike to happen.
>
> Reported-by: Friedrich Weber <f.weber@proxmox.com>
> Signed-off-by: Igor Thaller <igor.thaller@aon.at>
> ---
>  www/manager6/dc/Summary.js | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/www/manager6/dc/Summary.js b/www/manager6/dc/Summary.js
> index efb44dae..9b8f3280 100644
> --- a/www/manager6/dc/Summary.js
> +++ b/www/manager6/dc/Summary.js
> @@ -170,6 +170,11 @@ Ext.define('PVE.dc.Summary', {
>  			} else if (countedStorage[sid]) {
>  			    break;
>  			}
> +
> +			if (data.status === "unknown" && data.shared) {
> +			    break;
> +			}
> +
>  			used += data.disk;
>  			total += data.maxdisk;
>  			countedStorage[sid] = true;

lgtm :)


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel