You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by "acelyc111 (via GitHub)" <gi...@apache.org> on 2023/04/13 05:32:21 UTC
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1435: feat(new_metrics): add partition-level metric entity and migrate partition-level metrics for greedy_load_balancer of meta
acelyc111 commented on code in PR #1435:
URL: https://github.com/apache/incubator-pegasus/pull/1435#discussion_r1164978605
##########
src/meta/table_metrics.h:
##########
@@ -58,43 +137,55 @@ class table_metrics
METRIC_VAR_DECLARE_gauge_int64(unwritable_partitions);
METRIC_VAR_DECLARE_gauge_int64(writable_ill_partitions);
METRIC_VAR_DECLARE_gauge_int64(healthy_partitions);
- METRIC_VAR_DECLARE_counter(partition_configuration_changes);
- METRIC_VAR_DECLARE_counter(unwritable_partition_changes);
- METRIC_VAR_DECLARE_counter(writable_partition_changes);
+
+ std::vector<std::unique_ptr<partition_metrics>> _partition_metrics;
DISALLOW_COPY_AND_ASSIGN(table_metrics);
};
bool operator==(const table_metrics &lhs, const table_metrics &rhs);
bool operator!=(const table_metrics &lhs, const table_metrics &rhs);
-#define METRIC_DEFINE_TABLE_SET_METHOD(name, value_type) \
- void set_##name(int32_t table_id, value_type value) \
- { \
- utils::auto_read_lock l(_lock); \
- \
- entity_map::const_iterator iter = _entities.find(table_id); \
- if (dsn_unlikely(iter == _entities.end())) { \
- return; \
- } \
- METRIC_CALL_SET_METHOD(*(iter->second), name, value); \
- }
+class greedy_balance_stats
+{
+public:
+ greedy_balance_stats() = default;
+ ~greedy_balance_stats() = default;
+
+ struct partition_stats
+ {
+ int greedy_recent_balance_operations = 0;
+ int greedy_move_primary_operations = 0;
+ int greedy_copy_primary_operations = 0;
+ int greedy_copy_secondary_operations = 0;
+ };
-#define METRIC_CALL_TABLE_SET_METHOD(obj, name, table_id, value) (obj).set_##name(table_id, value)
+ using partition_map = std::unordered_map<gpid, partition_stats>;
Review Comment:
How about use *partition index* instead? It's 0 based and sequential, use it as the index of array would be faster.
##########
src/meta/table_metrics.h:
##########
@@ -21,35 +21,114 @@
#include <memory>
#include <unordered_map>
#include <utility>
+#include <vector>
+#include "common/gpid.h"
#include "utils/autoref_ptr.h"
+#include "utils/fmt_logging.h"
#include "utils/metrics.h"
#include "utils/ports.h"
#include "utils/synchronize.h"
namespace dsn {
-class table_metric_entities;
+
+// Maintain a partition-level metric entity of meta, and all metrics attached to it.
+class partition_metrics
+{
+public:
+ partition_metrics(int32_t table_id, int32_t partition_id);
+ ~partition_metrics() = default;
+
+ inline int32_t table_id() const { return _table_id; }
+ inline int32_t partition_id() const { return _partition_id; }
+ const metric_entity_ptr &partition_metric_entity() const;
+
+ METRIC_DEFINE_INCREMENT(partition_configuration_changes)
+ METRIC_DEFINE_INCREMENT(unwritable_partition_changes)
+ METRIC_DEFINE_INCREMENT(writable_partition_changes)
+
+ METRIC_DEFINE_SET(greedy_recent_balance_operations, int64_t)
+ METRIC_DEFINE_INCREMENT_BY(greedy_move_primary_operations)
+ METRIC_DEFINE_INCREMENT_BY(greedy_copy_primary_operations)
+ METRIC_DEFINE_INCREMENT_BY(greedy_copy_secondary_operations)
+
+private:
+ const int32_t _table_id;
+ const int32_t _partition_id;
+
+ const metric_entity_ptr _partition_metric_entity;
+ METRIC_VAR_DECLARE_counter(partition_configuration_changes);
+ METRIC_VAR_DECLARE_counter(unwritable_partition_changes);
+ METRIC_VAR_DECLARE_counter(writable_partition_changes);
+ METRIC_VAR_DECLARE_gauge_int64(greedy_recent_balance_operations);
+ METRIC_VAR_DECLARE_counter(greedy_move_primary_operations);
+ METRIC_VAR_DECLARE_counter(greedy_copy_primary_operations);
+ METRIC_VAR_DECLARE_counter(greedy_copy_secondary_operations);
+
+ DISALLOW_COPY_AND_ASSIGN(partition_metrics);
+};
+
+bool operator==(const partition_metrics &lhs, const partition_metrics &rhs);
+bool operator!=(const partition_metrics &lhs, const partition_metrics &rhs);
// Maintain a table-level metric entity of meta, and all metrics attached to it.
class table_metrics
{
public:
- table_metrics(int32_t table_id);
+ table_metrics(int32_t table_id, int32_t partition_count);
~table_metrics() = default;
inline int32_t table_id() const { return _table_id; }
const metric_entity_ptr &table_metric_entity() const;
- METRIC_DEFINE_SET_METHOD(dead_partitions, int64_t)
- METRIC_DEFINE_SET_METHOD(unreadable_partitions, int64_t)
- METRIC_DEFINE_SET_METHOD(unwritable_partitions, int64_t)
- METRIC_DEFINE_SET_METHOD(writable_ill_partitions, int64_t)
- METRIC_DEFINE_SET_METHOD(healthy_partitions, int64_t)
- METRIC_DEFINE_INCREMENT_METHOD(partition_configuration_changes)
- METRIC_DEFINE_INCREMENT_METHOD(unwritable_partition_changes)
- METRIC_DEFINE_INCREMENT_METHOD(writable_partition_changes)
+ void resize_partitions(int32_t partition_count);
+
+ METRIC_DEFINE_SET(dead_partitions, int64_t)
+ METRIC_DEFINE_SET(unreadable_partitions, int64_t)
+ METRIC_DEFINE_SET(unwritable_partitions, int64_t)
+ METRIC_DEFINE_SET(writable_ill_partitions, int64_t)
+ METRIC_DEFINE_SET(healthy_partitions, int64_t)
+
+#define __METRIC_DEFINE_INCREMENT_BY(name) \
+ void increment_by_##name(int32_t partition_id, int64_t x) \
Review Comment:
How about `increment_##name##_by` ?
##########
src/meta/table_metrics.cpp:
##########
@@ -17,12 +17,54 @@
#include "table_metrics.h"
+// IWYU pragma: no_include <ext/alloc_traits.h>
#include <fmt/core.h>
+#include <fmt/ostream.h>
+#include <stddef.h>
+#include <iosfwd>
#include <string>
#include "utils/fmt_logging.h"
#include "utils/string_view.h"
+METRIC_DEFINE_entity(partition);
+
+METRIC_DEFINE_counter(partition,
+ partition_configuration_changes,
+ dsn::metric_unit::kChanges,
+ "The number of times the configuration has been changed");
+
+METRIC_DEFINE_counter(partition,
+ unwritable_partition_changes,
+ dsn::metric_unit::kChanges,
+ "The number of times the status of partition has been changed to unwritable");
+
+METRIC_DEFINE_counter(partition,
+ writable_partition_changes,
+ dsn::metric_unit::kChanges,
+ "The number of times the status of partition has been changed to writable");
+
+METRIC_DEFINE_gauge_int64(
+ partition,
+ greedy_recent_balance_operations,
Review Comment:
Is the `recent` word necessary?
##########
src/meta/table_metrics.h:
##########
@@ -21,35 +21,114 @@
#include <memory>
#include <unordered_map>
#include <utility>
+#include <vector>
+#include "common/gpid.h"
#include "utils/autoref_ptr.h"
+#include "utils/fmt_logging.h"
#include "utils/metrics.h"
#include "utils/ports.h"
#include "utils/synchronize.h"
namespace dsn {
-class table_metric_entities;
+
+// Maintain a partition-level metric entity of meta, and all metrics attached to it.
+class partition_metrics
+{
+public:
+ partition_metrics(int32_t table_id, int32_t partition_id);
+ ~partition_metrics() = default;
+
+ inline int32_t table_id() const { return _table_id; }
+ inline int32_t partition_id() const { return _partition_id; }
+ const metric_entity_ptr &partition_metric_entity() const;
+
+ METRIC_DEFINE_INCREMENT(partition_configuration_changes)
+ METRIC_DEFINE_INCREMENT(unwritable_partition_changes)
+ METRIC_DEFINE_INCREMENT(writable_partition_changes)
+
+ METRIC_DEFINE_SET(greedy_recent_balance_operations, int64_t)
+ METRIC_DEFINE_INCREMENT_BY(greedy_move_primary_operations)
+ METRIC_DEFINE_INCREMENT_BY(greedy_copy_primary_operations)
+ METRIC_DEFINE_INCREMENT_BY(greedy_copy_secondary_operations)
+
+private:
+ const int32_t _table_id;
+ const int32_t _partition_id;
+
+ const metric_entity_ptr _partition_metric_entity;
+ METRIC_VAR_DECLARE_counter(partition_configuration_changes);
+ METRIC_VAR_DECLARE_counter(unwritable_partition_changes);
+ METRIC_VAR_DECLARE_counter(writable_partition_changes);
+ METRIC_VAR_DECLARE_gauge_int64(greedy_recent_balance_operations);
+ METRIC_VAR_DECLARE_counter(greedy_move_primary_operations);
+ METRIC_VAR_DECLARE_counter(greedy_copy_primary_operations);
+ METRIC_VAR_DECLARE_counter(greedy_copy_secondary_operations);
+
+ DISALLOW_COPY_AND_ASSIGN(partition_metrics);
+};
+
+bool operator==(const partition_metrics &lhs, const partition_metrics &rhs);
+bool operator!=(const partition_metrics &lhs, const partition_metrics &rhs);
// Maintain a table-level metric entity of meta, and all metrics attached to it.
class table_metrics
{
public:
- table_metrics(int32_t table_id);
+ table_metrics(int32_t table_id, int32_t partition_count);
~table_metrics() = default;
inline int32_t table_id() const { return _table_id; }
const metric_entity_ptr &table_metric_entity() const;
- METRIC_DEFINE_SET_METHOD(dead_partitions, int64_t)
- METRIC_DEFINE_SET_METHOD(unreadable_partitions, int64_t)
- METRIC_DEFINE_SET_METHOD(unwritable_partitions, int64_t)
- METRIC_DEFINE_SET_METHOD(writable_ill_partitions, int64_t)
- METRIC_DEFINE_SET_METHOD(healthy_partitions, int64_t)
- METRIC_DEFINE_INCREMENT_METHOD(partition_configuration_changes)
- METRIC_DEFINE_INCREMENT_METHOD(unwritable_partition_changes)
- METRIC_DEFINE_INCREMENT_METHOD(writable_partition_changes)
+ void resize_partitions(int32_t partition_count);
+
+ METRIC_DEFINE_SET(dead_partitions, int64_t)
+ METRIC_DEFINE_SET(unreadable_partitions, int64_t)
+ METRIC_DEFINE_SET(unwritable_partitions, int64_t)
+ METRIC_DEFINE_SET(writable_ill_partitions, int64_t)
+ METRIC_DEFINE_SET(healthy_partitions, int64_t)
+
+#define __METRIC_DEFINE_INCREMENT_BY(name) \
+ void increment_by_##name(int32_t partition_id, int64_t x) \
+ { \
+ CHECK_LT(partition_id, _partition_metrics.size()); \
+ METRIC_INCREMENT_BY(*(_partition_metrics[partition_id]), name, x); \
+ }
+
+ __METRIC_DEFINE_INCREMENT_BY(greedy_move_primary_operations)
+ __METRIC_DEFINE_INCREMENT_BY(greedy_copy_primary_operations)
+ __METRIC_DEFINE_INCREMENT_BY(greedy_copy_secondary_operations)
+
+#undef __METRIC_DEFINE_INCREMENT_BY
+
+#define __METRIC_DEFINE_INCREMENT(name) \
+ void increment_##name(int32_t partition_id) \
+ { \
+ CHECK_LT(partition_id, _partition_metrics.size()); \
Review Comment:
How about just `increment_##name##_by(partition_id, 1)`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org