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


##########
src/utils/metrics.cpp:
##########
@@ -41,17 +48,15 @@ metric_entity::~metric_entity()
     close(close_option::kWait);
 }
 
-void metric_entity::close(close_option option)
+void metric_entity::close(close_option option) const
 {
     utils::auto_write_lock l(_lock);
 
-    // The reason why each metric is closed in the entity rather than in the 
destructor of each
-    // metric is that close() for the metric will return immediately without 
waiting for any close
-    // operation to be finished.
-    //
-    // Thus, to close all metrics owned by an entity, it's more efficient to 
firstly issue a close
-    // request for all metrics; then, just wait for all of the close 
operations to be finished.
-    // It's inefficient to wait for each metric to be closed one by one.
+    // To close all metrics owned by an entity, it's more efficient to firstly 
issue an asynchronous
+    // close request to each metric; then, just wait for all of the close 
operations to be finished.
+    // It's inefficient to wait for each metric to be closed one by one. 
Therefore, the metric is
+    // not
+    // closed in its destructor.
     for (auto &m : _metrics) {

Review Comment:
   Compilation can be passed while this function is qualified with `const` 
since actually it does not modify the direct member (add, remove or update), 
such as `_metrics`; however, it will change the state of the member of 
`_metrics`. Therefore I think it's better to declare it without `const`.



-- 
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-unsubscr...@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@pegasus.apache.org
For additional commands, e-mail: dev-h...@pegasus.apache.org

Reply via email to