From e2e93803c5dc3b8d2871a13bee1ad583e615aac1 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Tue, 13 Apr 2021 17:32:17 +0530 Subject: [PATCH 1/5] server: Fix regression in storage pool stat update Fixes regression introduced in 71c5dbcf492a023dbea5f8c34f8fd883c3ad653f which would cause capacity bytes of certain pools to be update which shouldn't get updated by StatsCollector such as solidfire. Fixes #4911 Signed-off-by: Rohit Yadav --- .../java/com/cloud/server/StatsCollector.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index 86db544a196d..895df245abd7 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -1020,14 +1020,18 @@ protected void runInContext() { if (answer != null && answer.getResult()) { storagePoolStats.put(pool.getId(), (StorageStats)answer); + boolean poolNeedsUpdating = false; // Seems like we have dynamically updated the pool size since the prev. size and the current do not match - if (pool.getCapacityBytes() != ((StorageStats)answer).getCapacityBytes() || - pool.getUsedBytes() != ((StorageStats)answer).getByteUsed()) { + if (_storagePoolStats.get(poolId) != null && _storagePoolStats.get(poolId).getCapacityBytes() != ((StorageStats)answer).getCapacityBytes()) { pool.setCapacityBytes(((StorageStats)answer).getCapacityBytes()); - if (pool.getStorageProviderName().equalsIgnoreCase(DataStoreProvider.DEFAULT_PRIMARY)) { - pool.setUsedBytes(((StorageStats) answer).getByteUsed()); - pool.setUpdateTime(new Date()); - } + poolNeedsUpdating = true; + } + if (pool.getUsedBytes() != ((StorageStats)answer).getByteUsed() && pool.getStorageProviderName().equalsIgnoreCase(DataStoreProvider.DEFAULT_PRIMARY)) { + pool.setUsedBytes(((StorageStats) answer).getByteUsed()); + poolNeedsUpdating = true; + } + if (poolNeedsUpdating) { + pool.setUpdateTime(new Date()); _storagePoolDao.update(pool.getId(), pool); } } From a8b03d4bb305163944fc004bcd80746dca74e00a Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Tue, 13 Apr 2021 17:38:15 +0530 Subject: [PATCH 2/5] fix for case when value is 0, as many storage adapters return 0 as default Signed-off-by: Rohit Yadav --- server/src/main/java/com/cloud/server/StatsCollector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index 895df245abd7..3f367a19fe21 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -1026,7 +1026,7 @@ protected void runInContext() { pool.setCapacityBytes(((StorageStats)answer).getCapacityBytes()); poolNeedsUpdating = true; } - if (pool.getUsedBytes() != ((StorageStats)answer).getByteUsed() && pool.getStorageProviderName().equalsIgnoreCase(DataStoreProvider.DEFAULT_PRIMARY)) { + if (pool.getUsedBytes() != ((StorageStats)answer).getByteUsed() && pool.getStorageProviderName().equalsIgnoreCase(DataStoreProvider.DEFAULT_PRIMARY) && ((StorageStats)answer).getByteUsed() > 0) { pool.setUsedBytes(((StorageStats) answer).getByteUsed()); poolNeedsUpdating = true; } From 589d5e5599a9179c48c61799db7168882e2c9b8f Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 15 Apr 2021 14:17:14 +0530 Subject: [PATCH 3/5] address code review comments Signed-off-by: Rohit Yadav --- .../src/main/java/com/cloud/server/StatsCollector.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index 3f367a19fe21..91d6a6462912 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -1023,10 +1023,14 @@ protected void runInContext() { boolean poolNeedsUpdating = false; // Seems like we have dynamically updated the pool size since the prev. size and the current do not match if (_storagePoolStats.get(poolId) != null && _storagePoolStats.get(poolId).getCapacityBytes() != ((StorageStats)answer).getCapacityBytes()) { - pool.setCapacityBytes(((StorageStats)answer).getCapacityBytes()); - poolNeedsUpdating = true; + if (((StorageStats)answer).getCapacityBytes() > 0) { + pool.setCapacityBytes(((StorageStats)answer).getCapacityBytes()); + poolNeedsUpdating = true; + } else { + s_logger.warn("Received 0 capacity for pool ID " + poolId); + } } - if (pool.getUsedBytes() != ((StorageStats)answer).getByteUsed() && pool.getStorageProviderName().equalsIgnoreCase(DataStoreProvider.DEFAULT_PRIMARY) && ((StorageStats)answer).getByteUsed() > 0) { + if (pool.getUsedBytes() != ((StorageStats)answer).getByteUsed() && pool.getStorageProviderName().equalsIgnoreCase(DataStoreProvider.DEFAULT_PRIMARY) && !pool.isManaged()) { pool.setUsedBytes(((StorageStats) answer).getByteUsed()); poolNeedsUpdating = true; } From deb57edb2116ad1e0ff58a96b6022cb7cd40691c Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Fri, 16 Apr 2021 11:57:47 +0530 Subject: [PATCH 4/5] Update server/src/main/java/com/cloud/server/StatsCollector.java Co-authored-by: Nicolas Vazquez --- server/src/main/java/com/cloud/server/StatsCollector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index 91d6a6462912..ab6a166495e0 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -1027,7 +1027,7 @@ protected void runInContext() { pool.setCapacityBytes(((StorageStats)answer).getCapacityBytes()); poolNeedsUpdating = true; } else { - s_logger.warn("Received 0 capacity for pool ID " + poolId); + s_logger.warn("Not setting capacity bytes, received " + ((StorageStats)answer).getCapacityBytes() + " capacity for pool ID " + poolId); } } if (pool.getUsedBytes() != ((StorageStats)answer).getByteUsed() && pool.getStorageProviderName().equalsIgnoreCase(DataStoreProvider.DEFAULT_PRIMARY) && !pool.isManaged()) { From 2166d27302ce0ff32dc736c5013f278d2c468114 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Fri, 16 Apr 2021 11:58:46 +0530 Subject: [PATCH 5/5] Update StatsCollector.java --- server/src/main/java/com/cloud/server/StatsCollector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index ab6a166495e0..50f1063ad484 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -1030,7 +1030,7 @@ protected void runInContext() { s_logger.warn("Not setting capacity bytes, received " + ((StorageStats)answer).getCapacityBytes() + " capacity for pool ID " + poolId); } } - if (pool.getUsedBytes() != ((StorageStats)answer).getByteUsed() && pool.getStorageProviderName().equalsIgnoreCase(DataStoreProvider.DEFAULT_PRIMARY) && !pool.isManaged()) { + if (pool.getUsedBytes() != ((StorageStats)answer).getByteUsed() && pool.getStorageProviderName().equalsIgnoreCase(DataStoreProvider.DEFAULT_PRIMARY)) { pool.setUsedBytes(((StorageStats) answer).getByteUsed()); poolNeedsUpdating = true; }