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
[release-3.0] cherry pick: add commit log duration metrics #5881
[release-3.0] cherry pick: add commit log duration metrics #5881
Conversation
Signed-off-by: linning <linningde25@gmail.com>
Signed-off-by: linning <linningde25@gmail.com>
/run-all-tests |
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.
LGTM
if !ready.committed_entries.as_ref().map_or(true, Vec::is_empty) | ||
&& ctx.current_time.is_none() | ||
{ | ||
ctx.current_time.replace(monotonic_raw_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.
Why not initialize it at L1285?
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 look odd to initialize current_time
here, but it based on a reason:
when there are proposal msg current_time
will be initialize at L1650, and when there are not, initialize current_time
here is more close to initialize at L1650 than L1285, as between here and L1650 there is no time consume work like send msg or write to disk.
src/raftstore/store/peer.rs
Outdated
@@ -1275,6 +1281,9 @@ impl Peer { | |||
if lease_to_be_updated { | |||
let propose_time = self.find_propose_time(entry.get_index(), entry.get_term()); | |||
if let Some(propose_time) = propose_time { | |||
ctx.raft_metrics.commit_log.observe(duration_to_sec( |
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.
I think commit duration should be observed from the head instead of the tail.
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.
You are right, PTAL again.
Signed-off-by: linning <linningde25@gmail.com>
/run-all-tests |
* cherry-pick: add commit log duration metrics Signed-off-by: linning <linningde25@gmail.com> * cherry pick: rename lease_time to current_time Signed-off-by: linning <linningde25@gmail.com> * observe first propose_time Signed-off-by: linning <linningde25@gmail.com>
What have you changed?
cherry-pick: #5440 #5465
What is the type of the changes?
How is the PR tested?
No
Does this PR affect documentation (docs) or should it be mentioned in the release notes?
No
Does this PR affect
tidb-ansible
?pingcap/tidb-ansible#577