You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by "empiredan (via GitHub)" <gi...@apache.org> on 2023/04/12 12:13:19 UTC
[GitHub] [incubator-pegasus] empiredan opened a new pull request, #1435: feat(new_metrics): add partition-level metric entity and migrate partition-level metrics for load balancer of meta
empiredan opened a new pull request, #1435:
URL: https://github.com/apache/incubator-pegasus/pull/1435
https://github.com/apache/incubator-pegasus/issues/1331
--
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
[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
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
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
[GitHub] [incubator-pegasus] empiredan 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
Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #1435:
URL: https://github.com/apache/incubator-pegasus/pull/1435#discussion_r1165265711
##########
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:
Yes, actually this metric is updated after a batch of balance operations. Thus it is `recent`.
Also, it is somewhat special. It could be either the number of balance operations that have been executed, or the number of balance operations that would be executed in the future. As a result it can only be declared as gauge rather than counter.
--
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
[GitHub] [incubator-pegasus] empiredan 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
Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #1435:
URL: https://github.com/apache/incubator-pegasus/pull/1435#discussion_r1165249435
##########
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:
Means using 2-dimensional `std::unordered_map<int32_t, std::unordered_map<int32_t, partition_stats>` instead ?
--
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
[GitHub] [incubator-pegasus] empiredan 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
Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #1435:
URL: https://github.com/apache/incubator-pegasus/pull/1435#discussion_r1165303337
##########
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:
`increment_##name##_by(partition_id, 1)` will call `METRIC_VAR_INCREMENT_BY(...)`, which will check if the adder is zero. Thus better to call `increment_##name(...)` ?
```c++
#define METRIC_VAR_INCREMENT_BY(name, x) \
do { \
const auto v = (x); \
if (v != 0) { \
_##name->increment_by(v); \
} \
} while (0)
```
--
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
[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
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1435:
URL: https://github.com/apache/incubator-pegasus/pull/1435#discussion_r1165667692
##########
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:
Sorry, I think I made a mistake, just ignore it.
##########
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:
I see, thanks to the clarify.
--
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
[GitHub] [incubator-pegasus] acelyc111 merged pull request #1435: feat(new_metrics): add partition-level metric entity and migrate partition-level metrics for greedy_load_balancer of meta
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 merged PR #1435:
URL: https://github.com/apache/incubator-pegasus/pull/1435
--
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
[GitHub] [incubator-pegasus] empiredan 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
Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #1435:
URL: https://github.com/apache/incubator-pegasus/pull/1435#discussion_r1165385962
##########
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:
OK.
--
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