Record perf contexts to the write procedure #227
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #227 +/- ##
==========================================
+ Coverage 97.13% 97.22% +0.08%
==========================================
Files 30 30
Lines 9216 9368 +152
==========================================
+ Hits 8952 9108 +156
+ Misses 264 260 -4
Continue to review full report at Codecov.
|
tabokie
reviewed
Jun 22, 2022
sticnarf
force-pushed
the
perf-context
branch
2 times, most recently
from
5044f50
to
9501afe
Compare
Signed-off-by: Yilin Chen <sticnarf@gmail.com>
|
@sticnarf It's getting a little messy, I'm thinking something like this:
It basically decouples perf context from prometheus with a new interface |
tabokie
reviewed
Jun 23, 2022
Signed-off-by: Yilin Chen <sticnarf@gmail.com>
I think |
|
I think it important to pass the breakdown to the followers. For each thread, it still gives mostly accurate information about how long is spent inside raft-engine for each reason. |
Signed-off-by: Yilin Chen <sticnarf@gmail.com>
tabokie
reviewed
Jun 23, 2022
tabokie
reviewed
Jun 23, 2022
Signed-off-by: Yilin Chen <sticnarf@gmail.com>
Signed-off-by: Yilin Chen <sticnarf@gmail.com>
tabokie
reviewed
Jun 24, 2022
CHANGELOG.md
Outdated
| @@ -10,6 +10,7 @@ | |||
|
|
|||
| * Add `is_empty` to `Engine` API. | |||
| * Add metadata deletion capability to `FileSystem` trait. Users can implement `exists_metadata` and `delete_metadata` to clean up obsolete metadata from older versions of Raft Engine. | |||
| * Add `PerfContext` which records detailed time breakdown of the write process to thread-local storage. | |||
src/metrics.rs
Outdated
Comment on lines
49
to
50
| /// Time spent waiting for becoming the write leader. | ||
| pub write_leader_wait_duration: Duration, |
There was a problem hiding this comment.
Suggested change
| /// Time spent waiting for becoming the write leader. | |
| pub write_leader_wait_duration: Duration, | |
| /// Time spent waiting for the write group to form and get processed. | |
| pub write_wait_duration: Duration, |
| } | ||
| let entered_time = writer.entered_time.unwrap(); | ||
| ENGINE_WRITE_PREPROCESS_DURATION_HISTOGRAM | ||
| .observe(entered_time.saturating_duration_since(start).as_secs_f64()); |
There was a problem hiding this comment.
debug assert perf_context_diff.write_wait_duration == 0.
Signed-off-by: Yilin Chen <sticnarf@gmail.com>
tabokie
approved these changes
Jun 24, 2022
ti-chi-bot
added a commit
to tikv/tikv
that referenced
this pull request
Jun 28, 2022
ref tikv/raft-engine#227, ref #12362 We used to record perf contexts for Raft RocksDB and KV RocksDB with the same PerfContext. But we also have raft-engine now. So, we will miss perf contexts if we still use RocksDB perf contexts. This commit adds PerfContext support to RaftEngine and distinguish it from the perf context used for applying. Then, we'll record correct perf statistics for both raft engine and KV DB. Updated raft-engine to include tikv/raft-engine#227 Signed-off-by: Yilin Chen <sticnarf@gmail.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
ethercflow
pushed a commit
to ethercflow/tikv
that referenced
this pull request
Jun 28, 2022
ref tikv/raft-engine#227, ref tikv#12362 We used to record perf contexts for Raft RocksDB and KV RocksDB with the same PerfContext. But we also have raft-engine now. So, we will miss perf contexts if we still use RocksDB perf contexts. This commit adds PerfContext support to RaftEngine and distinguish it from the perf context used for applying. Then, we'll record correct perf statistics for both raft engine and KV DB. Updated raft-engine to include tikv/raft-engine#227 Signed-off-by: Yilin Chen <sticnarf@gmail.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR tries to add RocksDB-like perf contexts to Raft Engine.
Now, I only add these statistics:
log_populating_nanoswrite_leader_wait_nanoslog_write_nanoslog_rotate_nanoslog_sync_nanoswrite_apply_nanosI'm willing to know if there are other important metrics that is worth recording here.