You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by GitBox <gi...@apache.org> on 2023/01/12 07:08:49 UTC

[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1304: feat(new_metrics): retire the stale metrics and entities that are no longer used

acelyc111 commented on code in PR #1304:
URL: https://github.com/apache/incubator-pegasus/pull/1304#discussion_r1067769180


##########
src/utils/metrics.cpp:
##########
@@ -177,6 +203,109 @@ void metric_entity::take_snapshot(metric_json_writer &writer, const metric_filte
     writer.EndObject();
 }
 
+bool metric_entity::is_stale() const
+{
+    // Since this entity itself is still being accessed, its reference count should be 1
+    // at least.
+    CHECK_GE(get_count(), 1);
+
+    // Once this entity did not have any metric, and had only one reference kept in the
+    // registry, this entity would be considered useless.
+    return _metrics.empty() && get_count() == 1;
+}
+
+metric_entity::collected_stale_metrics_info metric_entity::collect_stale_metrics() const
+{
+    collected_stale_metrics_info collected_info;
+
+    auto now = dsn_now_ms();
+
+    utils::auto_read_lock l(_lock);
+
+    for (const auto &m : _metrics) {
+        if (!m.second->is_stale()) {

Review Comment:
   I'm a bit of confused why only partial metrics of the entity are stale, aren't all of  them become stale at the same time when the entity they belong to is destroyed?



##########
src/utils/metrics.cpp:
##########
@@ -177,6 +203,109 @@ void metric_entity::take_snapshot(metric_json_writer &writer, const metric_filte
     writer.EndObject();
 }
 
+bool metric_entity::is_stale() const
+{
+    // Since this entity itself is still being accessed, its reference count should be 1
+    // at least.
+    CHECK_GE(get_count(), 1);
+
+    // Once this entity did not have any metric, and had only one reference kept in the
+    // registry, this entity would be considered useless.

Review Comment:
   How about use `stale` instead of `useless`?



-- 
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