acelyc111 commented on code in PR #1074:
URL: https://github.com/apache/incubator-pegasus/pull/1074#discussion_r930910435
##########
src/rdsn/src/utils/metrics.cpp:
##########
@@ -132,27 +185,78 @@ uint64_t
percentile_timer::generate_initial_delay_ms(uint64_t interval_ms)
return (rand::next_u64() % interval_seconds + 1) * 1000 + rand::next_u64()
% 1000;
}
-percentile_timer::percentile_timer(uint64_t interval_ms, exec_fn exec)
+percentile_timer::percentile_timer(uint64_t interval_ms, on_exec_fn on_exec,
on_close_fn on_close)
: _initial_delay_ms(generate_initial_delay_ms(interval_ms)),
_interval_ms(interval_ms),
- _exec(exec),
+ _on_exec(on_exec),
+ _on_close(on_close),
+ _state(state::kRunning),
+ _completed(),
_timer(new
boost::asio::deadline_timer(tools::shared_io_service::instance().ios))
{
_timer->expires_from_now(boost::posix_time::milliseconds(_initial_delay_ms));
_timer->async_wait(std::bind(&percentile_timer::on_timer, this,
std::placeholders::_1));
}
+void percentile_timer::close()
+{
+ // If the timer has already expired when cancel() is called, then the
handlers for asynchronous
+ // wait operations will:
+ // * have already been invoked; or
+ // * have been queued for invocation in the near future.
+ //
+ // These handlers can no longer be cancelled, and therefore are passed an
error code that
+ // indicates the successful completion of the wait operation. Thus set the
state of timer to
+ // kClosing to tell on_timer() that the timer should be closed even if it
is not called with
+ // operation_canceled.
+ auto expected_state = state::kRunning;
+ if (_state.compare_exchange_strong(expected_state, state::kClosing)) {
+ _timer->cancel();
+ }
+}
+
+void percentile_timer::wait() { _completed.wait(); }
+
+void percentile_timer::on_close()
+{
+ _on_close();
+ _completed.notify();
+}
+
void percentile_timer::on_timer(const boost::system::error_code &ec)
{
+// This macro is defined for the case that handlers for asynchronous wait
operations are no
+// longer cancelled. It just checks the internal state atomically (since
close() can also be
+// called simultaneously) for kClosing; once it's matched, it will not execute
future callbacks
+// to stop the timer.
+#define TRY_PROCESS_TIMER_CLOSING()
\
+ do {
\
+ auto expected_state = state::kClosing;
\
+ if (_state.compare_exchange_strong(expected_state, state::kClosed)) {
\
+ on_close();
\
+ return;
\
+ }
\
+ } while (0)
+
if (dsn_unlikely(!!ec)) {
dassert_f(ec == boost::system::errc::operation_canceled,
"failed to exec on_timer with an error that cannot be
handled: {}",
ec.message());
+
+ // Cancel can only be launched by close().
+ auto expected_state = state::kClosing;
+ dassert_f(_state.compare_exchange_strong(expected_state,
state::kClosed),
+ "wrong state for percentile_timer: {}, while expecting
closing state",
+ static_cast<int>(expected_state));
+ on_close();
+
return;
}
- _exec();
+ TRY_PROCESS_TIMER_CLOSING();
+ _on_exec();
+ TRY_PROCESS_TIMER_CLOSING();
Review Comment:
better to add `#undef TRY_PROCESS_TIMER_CLOSING` when you'll never use it.
##########
src/rdsn/include/dsn/utility/metrics.h:
##########
@@ -168,6 +171,16 @@ class metric_entity : public ref_counter
~metric_entity();
+ // Close all "closeable" metrics owned by this entity.
+ //
+ // `async` is used to control if the close operations are asynchronous or
not. It is set to
+ // true by default, which means all metrics owned by this entity will be
closed asynchronously
+ // without waiting for the close operations to be finished.
+ //
+ // Otherwise, once `async` is set to false, close() will be blocked until
the close operations
+ // are finished.
+ void close(bool async = true);
Review Comment:
use 'wait' maybe more accurate, and it's suggest to use enum class instead
of boolean, true/false is meanless when we see the invoker.
--
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]
