[incubator-pegasus] branch migrate-metrics-dev updated: fix(new_metrics): total_capacity_mb and total_available_mb are not atomic (#1542)

[la...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

laiyingchun pushed a commit to branch migrate-metrics-dev
in repository https://gitbox.apache.org/repos/asf/incubator-pegasus.git


The following commit(s) were added to refs/heads/migrate-metrics-dev by this push:
     new 1a3ab78fb fix(new_metrics): total_capacity_mb and total_available_mb are not atomic (#1542)
1a3ab78fb is described below

commit 1a3ab78fb293a0aa40b3806549808e0bf837447e
Author: Dan Wang <wa...@apache.org>
AuthorDate: Sun Jun 25 12:11:59 2023 +0800

    fix(new_metrics): total_capacity_mb and total_available_mb are not atomic (#1542)
---
 src/block_service/hdfs/hdfs_service.cpp |  1 -
 src/common/fs_manager.cpp               | 17 ++++++++++-------
 src/common/fs_manager.h                 |  5 +++--
 src/replica/replica_stub.cpp            |  4 ++--
 src/replica/test/replica_test.cpp       |  1 -
 5 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/src/block_service/hdfs/hdfs_service.cpp b/src/block_service/hdfs/hdfs_service.cpp
index 459108e11..72b520028 100644
--- a/src/block_service/hdfs/hdfs_service.cpp
+++ b/src/block_service/hdfs/hdfs_service.cpp
@@ -19,7 +19,6 @@
 #include <fcntl.h>
 #include <algorithm>
 #include <fstream>
-#include <type_traits>
 #include <utility>
 
 #include "block_service/directio_writable_file.h"
diff --git a/src/common/fs_manager.cpp b/src/common/fs_manager.cpp
index 9b8d791e6..f33e9eece 100644
--- a/src/common/fs_manager.cpp
+++ b/src/common/fs_manager.cpp
@@ -376,8 +376,8 @@ void fs_manager::remove_replica(const gpid &pid)
 
 void fs_manager::update_disk_stat()
 {
-    _total_capacity_mb = 0;
-    _total_available_mb = 0;
+    int64_t total_capacity_mb = 0;
+    int64_t total_available_mb = 0;
     int total_available_ratio = 0;
     int min_available_ratio = 100;
     int max_available_ratio = 0;
@@ -393,23 +393,26 @@ void fs_manager::update_disk_stat()
             continue;
         }
         dn->update_disk_stat();
-        _total_capacity_mb += dn->disk_capacity_mb;
-        _total_available_mb += dn->disk_available_mb;
+        total_capacity_mb += dn->disk_capacity_mb;
+        total_available_mb += dn->disk_available_mb;
         min_available_ratio = std::min(dn->disk_available_ratio, min_available_ratio);
         max_available_ratio = std::max(dn->disk_available_ratio, max_available_ratio);
     }
     total_available_ratio = static_cast<int>(
-        _total_capacity_mb == 0 ? 0 : std::round(_total_available_mb * 100.0 / _total_capacity_mb));
+        total_capacity_mb == 0 ? 0 : std::round(total_available_mb * 100.0 / total_capacity_mb));
 
     LOG_INFO("update disk space succeed: disk_count = {}, total_capacity_mb = {}, "
              "total_available_mb = {}, total_available_ratio = {}%, min_available_ratio = {}%, "
              "max_available_ratio = {}%",
              _dir_nodes.size(),
-             _total_capacity_mb,
-             _total_available_mb,
+             total_capacity_mb,
+             total_available_mb,
              total_available_ratio,
              min_available_ratio,
              max_available_ratio);
+
+    _total_capacity_mb.store(total_capacity_mb, std::memory_order_relaxed);
+    _total_available_mb.store(total_available_mb, std::memory_order_relaxed);
 }
 
 void fs_manager::add_new_dir_node(const std::string &data_dir, const std::string &tag)
diff --git a/src/common/fs_manager.h b/src/common/fs_manager.h
index c0c461c57..eacdeb317 100644
--- a/src/common/fs_manager.h
+++ b/src/common/fs_manager.h
@@ -29,6 +29,7 @@
 #include "common/replication_other_types.h"
 #include "metadata_types.h"
 #include "utils/autoref_ptr.h"
+#include "utils/error_code.h"
 #include "utils/flags.h"
 #include "utils/metrics.h"
 #include "utils/ports.h"
@@ -170,8 +171,8 @@ private:
     // Especially when visiting the holding_replicas, you must take care.
     mutable zrwlock_nr _lock; // [ lock
 
-    int64_t _total_capacity_mb = 0;
-    int64_t _total_available_mb = 0;
+    std::atomic<int64_t> _total_capacity_mb{0};
+    std::atomic<int64_t> _total_available_mb{0};
 
     // Once dir_node has been added to '_dir_nodes', it will not be removed, it will be marked
     // as non-NORMAL status if it is not available.
diff --git a/src/replica/replica_stub.cpp b/src/replica/replica_stub.cpp
index 292af639c..d917625bb 100644
--- a/src/replica/replica_stub.cpp
+++ b/src/replica/replica_stub.cpp
@@ -910,8 +910,8 @@ void replica_stub::on_query_disk_info(query_disk_info_rpc rpc)
 
     resp.disk_infos = _fs_manager.get_disk_infos(app_id);
     // Get the statistics from fs_manager's metrics, they are thread-safe.
-    resp.total_capacity_mb = _fs_manager._counter_total_capacity_mb->get_integer_value();
-    resp.total_available_mb = _fs_manager._counter_total_available_mb->get_integer_value();
+    resp.total_capacity_mb = _fs_manager._total_capacity_mb.load(std::memory_order_relaxed);
+    resp.total_available_mb = _fs_manager._total_available_mb.load(std::memory_order_relaxed);
     resp.err = ERR_OK;
 }
 
diff --git a/src/replica/test/replica_test.cpp b/src/replica/test/replica_test.cpp
index f2a211384..8a4d215c1 100644
--- a/src/replica/test/replica_test.cpp
+++ b/src/replica/test/replica_test.cpp
@@ -22,7 +22,6 @@
 #include <gtest/gtest.h>
 #include <stddef.h>
 #include <stdint.h>
-#include <unistd.h>
 #include <atomic>
 #include <iostream>
 #include <map>


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pegasus.apache.org
For additional commands, e-mail: commits-help@pegasus.apache.org