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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
