This is an automated email from the ASF dual-hosted git repository. wangdan pushed a commit to branch migrate-metrics-dev in repository https://gitbox.apache.org/repos/asf/incubator-pegasus.git
commit fa0767f880b72d91d918e3995e387d7b6f04d59c Author: Dan Wang <[email protected]> AuthorDate: Sun Jun 25 12:11:59 2023 +0800 fix(new_metrics): total_capacity_mb/total_available_mb are not atomic (#1542) https://github.com/apache/incubator-pegasus/issues/1541 After rebasing from branch master onto migrate-metrics-dev, there are some problems found for metrics total_capacity_mb/total_available_mb of fs_manager: - both are referencing the value of perf-counters which have been changed to new framework and cannot be compiled successfully; - setting/getting both metrics are not atomic. This PR is to solve both problems. --- 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 d5aa20778..228c54903 100644 --- a/src/replica/replica_stub.cpp +++ b/src/replica/replica_stub.cpp @@ -921,8 +921,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: [email protected] For additional commands, e-mail: [email protected]
