-
Notifications
You must be signed in to change notification settings - Fork 5.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
common: use mono clock for HeartbeatMap #17213
Conversation
s/heaerbeatmap/HeartbeatMap/ |
e5b8136
to
32cb435
Compare
src/common/HeartbeatMap.h
Outdated
@@ -79,12 +80,12 @@ class HeartbeatMap { | |||
private: | |||
CephContext *m_cct; | |||
RWLock m_rwlock; | |||
time_t m_inject_unhealthy_until; | |||
unsigned m_inject_unhealthy_until; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this is a time_point instead wouldn't that avoid most of the casts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree, would be easier if this is a time_point
.
src/common/HeartbeatMap.h
Outdated
std::list<heartbeat_handle_d*> m_workers; | ||
std::atomic<unsigned> m_unhealthy_workers = { 0 }; | ||
std::atomic<unsigned> m_total_workers = { 0 }; | ||
|
||
bool _check(const heartbeat_handle_d *h, const char *who, time_t now); | ||
bool _check(const heartbeat_handle_d *h, const char *who, ceph::coarse_mono_clock::time_point& now); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pass by const ceph::coarse_mono_clock::time_point&
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with kefu.
src/common/HeartbeatMap.cc
Outdated
|
||
was = h->timeout; | ||
if (was && was < now) { | ||
if (was && was < std::chrono::time_point_cast<std::chrono::seconds>(now).time_since_epoch().count()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be good to use typdef for 'std::chrono::time_point_caststd::chrono::seconds' instead repeating again & again
@XinzeChi ping. |
32cb435
to
095646b
Compare
@tchaikov pong |
src/common/HeartbeatMap.h
Outdated
@@ -79,12 +80,12 @@ class HeartbeatMap { | |||
private: | |||
CephContext *m_cct; | |||
RWLock m_rwlock; | |||
time_t m_inject_unhealthy_until; | |||
unsigned m_inject_unhealthy_until; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree, would be easier if this is a time_point
.
_check(h, "reset_timeout", now); | ||
|
||
h->timeout = now + grace; | ||
h->timeout = tp_to_seconds(now) + grace; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might want to use std::atomic<ceph::coarse_mono_clock>
for heartbeat_handle_d::timeout
and heartbeat_handle_d::suicide_timeout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks complicated if we set std::atomicceph::coarse_mono_clock. https://stackoverflow.com/questions/29364456/stdatomicstdchronohigh-resolution-clocktime-point-can-not-compile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@XinzeChi sorry, i just figured out that this is a bug in GCC. see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53901 and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53903.
Signed-off-by: Xinze Chi <xinze@xsky.com>
095646b
to
e96eb96
Compare
closing in favor of #17827 |
Signed-off-by: Xinze Chi xinze@xsky.com