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 2022/08/15 03:53:27 UTC

[GitHub] [incubator-pegasus] empiredan opened a new pull request, #1117: feat: introduce data sink to collect snapshot from each metric periodically

empiredan opened a new pull request, #1117:
URL: https://github.com/apache/incubator-pegasus/pull/1117

   This PR is for https://github.com/apache/incubator-pegasus/issues/1116.
   
   This PR introduces data sink to collect snapshot from each metric periodically. The base data sink is actually an abstract class that has interfaces needed by concrete monitoring systems. The unit test has implemented a data sink and verified if the snapshots can be received correctly.


-- 
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 #1117: feat: introduce data sink to collect snapshot from each metric periodically

Posted by GitBox <gi...@apache.org>.
acelyc111 commented on code in PR #1117:
URL: https://github.com/apache/incubator-pegasus/pull/1117#discussion_r949857330


##########
src/rdsn/include/dsn/utility/metrics.h:
##########
@@ -134,10 +135,13 @@ class metric_prototype;
 class metric;
 using metric_ptr = ref_ptr<metric>;
 
+class metric_data_sink;
+using metric_data_sink_ptr = ref_ptr<metric_data_sink>;
+
 class metric_entity : public ref_counter
 {
 public:
-    using attr_map = std::unordered_map<std::string, std::string>;
+    using attr_map = std::map<std::string, std::string>;

Review Comment:
   why change to use std:map?



##########
src/rdsn/include/dsn/utility/metrics.h:
##########
@@ -215,6 +221,54 @@ class metric_entity_prototype
     DISALLOW_COPY_AND_ASSIGN(metric_entity_prototype);
 };
 
+// `metric_timer` is a timer class that runs metric-related computations periodically, such as

Review Comment:
   > runs metric-related computations periodically
   
   Even though there isn't any thirdparty monitoring system, it will still caculate periodically? Would it be a bit of cost?
   Another way is to calculate lazily, that is to say, calculate will only be triggered when any body request these metrics, you can define some callbacks for that.



-- 
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 #1117: feat: introduce data sink to collect snapshot from each metric periodically

Posted by GitBox <gi...@apache.org>.
empiredan commented on code in PR #1117:
URL: https://github.com/apache/incubator-pegasus/pull/1117#discussion_r956689572


##########
src/rdsn/include/dsn/utility/metrics.h:
##########
@@ -134,10 +135,13 @@ class metric_prototype;
 class metric;
 using metric_ptr = ref_ptr<metric>;
 
+class metric_data_sink;
+using metric_data_sink_ptr = ref_ptr<metric_data_sink>;
+
 class metric_entity : public ref_counter
 {
 public:
-    using attr_map = std::unordered_map<std::string, std::string>;
+    using attr_map = std::map<std::string, std::string>;

Review Comment:
   All of the attributes of an entity will be used as the labels of the metric. The labels will later be passed to `metric_snapshot`, thus each metric snapshot may has multiple labels. To maintain each snapshot in a map, data sink may choose to encode the labels a snapshot has, as what `metric_snapshot::encode_attributes` has done. In comparison with `std::unordered_map`, it is easier for `std::map` to be encoded since it's ordered.



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