You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pegasus.apache.org by wa...@apache.org on 2023/06/25 08:15:03 UTC

[incubator-pegasus] branch migrate-metrics-dev updated (1a3ab78fb -> eee906755)

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

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


    omit 1a3ab78fb fix(new_metrics): total_capacity_mb and total_available_mb are not atomic (#1542)
     new eee906755 fix(new_metrics): total_capacity_mb/total_available_mb are not atomic (#1542)

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (1a3ab78fb)
            \
             N -- N -- N   refs/heads/migrate-metrics-dev (eee906755)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:


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


[incubator-pegasus] 01/01: fix(new_metrics): total_capacity_mb/total_available_mb are not atomic (#1542)

Posted by wa...@apache.org.
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 eee906755e5fd13fcf01459948725f85d1699ae2
Author: Dan Wang <wa...@apache.org>
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 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