Skip to content
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

Performance Diagnosis Enhancements #12362

Open
9 of 40 tasks
sticnarf opened this issue Apr 14, 2022 · 3 comments
Open
9 of 40 tasks

Performance Diagnosis Enhancements #12362

sticnarf opened this issue Apr 14, 2022 · 3 comments
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@sticnarf
Copy link
Contributor

sticnarf commented Apr 14, 2022

TiKV users and developers often care about performance, but it is never easy to diagnose performance issues or find performance bottlenecks.

  • Latency of some parts in the critical path is not recorded, sometimes making the cause of performance issues only inferred indirectly.
  • Metrics displayed in the Grafana dashboard are not very well organized. It's not easy to find the cause quickly.
  • It's hard to investigate the cause of an individual slow request if the overall latency is normal.

To improve the situation, we need to fill up the missing parts of the metrics, add more details to the metrics and try to improve the way we display the metrics. And, it's better to collect performance data in the request level and send back to the client if possible, enabling analyzing tail latency causes.

Data to be collected

Below are the missing data we are going to collect. "(R)" means the data may be collected for each request, and "(M)" means they are collected as metrics.

Read

  • (R) Whether async_snapshot is completed by the local reader
  • (R) async_snapshot propose wait duration
  • (R) async_snapshot read index wait duration
  • (RM) RocksDB GetSnapshot duration
  • (R) RocksDB seek count, total seek duration, max seek duration
  • (R) RocksDB block read duration
  • (RM) RocksDB DB mutex wait duration
  • (RM) Read pool scheduling wait duration
  • (RM) Read pool queue length
  • (R) Priority in the read pool
  • (RM) Semaphore wait duration
  • (RM) Semaphore wait queue length
  • (RM) Task yield count, total yield duration

Write

  • (RM) Latch wait queue length
  • (R) Latch wait time
  • (RM) Scheduler pool wait duration
  • (RM) Scheduler pool wait queue length
  • (R) Scheduler flow control throttle duration
  • (R) Scheduler flow control throttle reason
  • (R) Request async_write duration
  • (R) Request propose wait duration
  • (RM) Store loop queue length
  • (RM) Store loop queue wait rounds
  • (R) Store loop batch size
  • (R) Store loop batch write size
  • (R) Write Raft log duration (including RocksDB details)
  • (RM) Raft flow control duration
  • (RM) Inter-TiKV RPC latency
  • (R) Apply wait duration
  • (R) Read entry cache duration (if happens)
  • (RM) Apply loop queue length
  • (RM) Apply loop wait rounds
  • (R) Write KV RocksDB duration (including RocksDB details)

Background jobs

Internal jobs from TiDB (ref pingcap/tidb#33963)

  • (M) TiDB auto-analyze CPU and I/O usage
  • (M) TiDB DDL reorganization (add index/change column) CPU and I/O usage, raftstore commands ratio
  • (M) Checksum CPU and I/O usage

Diagnosis helper

The performance overview dashboard displays performance critical metrics. Some representitive newly added metrics can be put on the same dashboard as well.

Smart diagnosis service may use the metrics as input and generate reports about possible cause of the problem. But this is not in the TiKV project plan AFAIK.

Existing related issues

The following issues were created before, they may be some sub-tasks of this meta-issue. We hope this issue will cover most of the problems raised by the following issues.

@sticnarf sticnarf added the type/enhancement The issue or PR belongs to an enhancement. label Apr 14, 2022
ti-chi-bot added a commit that referenced this issue Apr 20, 2022
ref #12362

PerfFlags provides more fine-grained PerfContext collecting. In this way, we
only collect the duration we really care about. Theoretically, it reduces the
overhead of collecting PerfContext compared to using PerfLevel::EnableTime.

This commit only changes the raftstore part. The read pool part will be done
later.

The default value of raftstore.perf-level is changed to Uninitialized. The
config is not documented before, so I think the change is not so important
to change the default value.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
ti-chi-bot pushed a commit that referenced this issue May 6, 2022
…2432)

ref #12362

This commit enables some more perf flags about reading, such as
block read time, DB mutex wait time. So, it enables us to find out
the root cause of more performance issues.

It also includes some refactorings of unifying the functions of capturing
PerfContext in tikv_kv into engine_rocks because engine specific functions
should be hidden in engine_* as much as possible. This is the first step that
removes statistics from tikv_kv, and later code referencing directly to the
engine_rocks should be also removed.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>
3AceShowHand pushed a commit to 3AceShowHand/tikv that referenced this issue May 7, 2022
…kv#12432)

ref tikv#12362

This commit enables some more perf flags about reading, such as
block read time, DB mutex wait time. So, it enables us to find out
the root cause of more performance issues.

It also includes some refactorings of unifying the functions of capturing
PerfContext in tikv_kv into engine_rocks because engine specific functions
should be hidden in engine_* as much as possible. This is the first step that
removes statistics from tikv_kv, and later code referencing directly to the
engine_rocks should be also removed.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>
Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>
ti-chi-bot added a commit that referenced this issue Jun 6, 2022
ref #12362

This commit measures the read duration spent on reading (e.g. write
conflict checks). This fixes the missing part of scheduler commands
and may help diagnosis when scheduler reading takes a long time.

This commit also changes some now_coarse to now. now_coarse
has a precision of 10ms on many systems, so it may not meet our
precision requirement. Instant::now is fast enough to be called in
these cases.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
BornChanger pushed a commit to BornChanger/tikv that referenced this issue Jun 10, 2022
ref tikv#12362

This commit measures the read duration spent on reading (e.g. write
conflict checks). This fixes the missing part of scheduler commands
and may help diagnosis when scheduler reading takes a long time.

This commit also changes some now_coarse to now. now_coarse
has a precision of 10ms on many systems, so it may not meet our
precision requirement. Instant::now is fast enough to be called in
these cases.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
ti-chi-bot pushed a commit that referenced this issue Jun 10, 2022
ref #12362

This commit is a large refactoring that moves collecting engine
PerfContext from storage and coprocessor to engine_rocks and
the tracker.

Now, the storage and coprocessor are mostly decoupled with a
specific engine (engine_rocks).

And it introduces a general trakcer mechanism to collect the metrics
of a request during its whole lifetime. It will help us collect more
performance critical data of a single request more easily.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>
BornChanger pushed a commit to BornChanger/tikv that referenced this issue Jun 10, 2022
ref tikv#12362

This commit measures the read duration spent on reading (e.g. write
conflict checks). This fixes the missing part of scheduler commands
and may help diagnosis when scheduler reading takes a long time.

This commit also changes some now_coarse to now. now_coarse
has a precision of 10ms on many systems, so it may not meet our
precision requirement. Instant::now is fast enough to be called in
these cases.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
ti-chi-bot added a commit that referenced this issue Jun 16, 2022
ref #12362

This commit records perf contexts around executing txn
scheduler commands. This helps us know the detail performance
data of the underlying engine.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
ti-chi-bot pushed a commit that referenced this issue Jun 20, 2022
ref #12362

Our raft implementation has its flow control mechanism to limit the inflight
message number. But we're not able to know whether and when we are
throttled. Then, it's hard for us to know whether we should adjust the
max-inflight-msgs config.

This commit tries to improve this case. It's complex to add hooks to raft-rs
to know how long a message is throttled, but we can do it in the raftstore.
We record the propose time, and consume it and record it in the histogram
when the messages is send through the transport. If flow control takes effect,
the ready will be smaller than all under-replicated logs. So, we can know if
flow control takes effect.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>
ti-chi-bot pushed a commit that referenced this issue Jun 24, 2022
ref #12362

This commit replaces the request_times in the raftstore callback with a
tracker token. Then, the waterfall metrics of a raft command will be
recorded into the tracker.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>
ti-chi-bot added a commit that referenced this issue 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 issue Jun 28, 2022
ref tikv#12362

This commit replaces the request_times in the raftstore callback with a
tracker token. Then, the waterfall metrics of a raft command will be
recorded into the tracker.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>
ethercflow pushed a commit to ethercflow/tikv that referenced this issue 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>
ti-chi-bot added a commit that referenced this issue Jul 5, 2022
ref #12362

TiKV client can pass request_source through Context. It is
useful for us to know how many requests there are from each
source.

So, this commit collects the count and the total duration by
request source. The source label is not added to the command
type in order to avoid creating too many label combinations.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
ti-chi-bot added a commit that referenced this issue Jul 11, 2022
ref #931, ref #12362

kvproto#931 adds a few more details about the time used
by an RPC request. We are filling these additional information
to the responses in this commit.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
@sticnarf
Copy link
Contributor Author

@OneSizeFitsQuorum Are you interested in the time breakdown inside async_snapshot? It may be useful to collect whether lease is expired and record the raft store loop wait duration if lease is expired.

@OneSizeFitsQuorum
Copy link
Contributor

Yes, I can practice on it first. But I’m curious about what is Apply loop Queue Length and Apply loop wait rounds, will them be helpful for us to monitor the the applying status? If so it maybe useful for parallel apply demo too.

@sticnarf
Copy link
Contributor Author

Yes, I can practice on it first. But I’m curious about what is Apply loop Queue Length and Apply loop wait rounds, will them be helpful for us to monitor the the applying status? If so it maybe useful for parallel apply demo too.

I was thinking Apply loop Queue Length as the total number of commands that is pending to be applied at the time when a new command is committed and sent to the apply threads. And meanwhile, we can record the value of the current "apply round" counter. The counter increases for each apply batch. So, we can know how many rounds of batches have passed before we are able to handle the command in the apply thread.

Probably they are helpful to account for the long apply wait time while the apply threads are vacant. But I am also a bit worried that these metrics are too tedious in most cases. (Maybe we can define levels for metrics too.)

ti-chi-bot pushed a commit that referenced this issue Aug 22, 2022
ref pingcap/kvproto#965, ref #12362

This commit record the pessimistic_lock_wait time for pessimistic transactions in the
waitManager.

Signed-off-by: OneSizeFitQuorum <tanxinyu@apache.org>
ti-chi-bot pushed a commit that referenced this issue Aug 29, 2022
ref pingcap/kvproto#974, ref pingcap/kvproto#978, ref #12362

This commit records read_index_propose_wait_nanos, read_index_confirm_wait_nanos and read_pool_schedule_wait_nanos

Signed-off-by: OneSizeFitQuorum <tanxinyu@apache.org>
TonsnakeLin pushed a commit to TonsnakeLin/tikv that referenced this issue Aug 29, 2022
ref pingcap/kvproto#974, ref pingcap/kvproto#978, ref tikv#12362

This commit records read_index_propose_wait_nanos, read_index_confirm_wait_nanos and read_pool_schedule_wait_nanos

Signed-off-by: OneSizeFitQuorum <tanxinyu@apache.org>
Signed-off-by: TonsnakeLin <lpbgytong@163.com>
fengou1 pushed a commit to fengou1/tikv that referenced this issue Aug 30, 2022
ref pingcap/kvproto#965, ref tikv#12362

This commit record the pessimistic_lock_wait time for pessimistic transactions in the
waitManager.

Signed-off-by: OneSizeFitQuorum <tanxinyu@apache.org>
Signed-off-by: fengou1 <feng.ou@pingcap.com>
fengou1 pushed a commit to fengou1/tikv that referenced this issue Aug 30, 2022
ref pingcap/kvproto#974, ref pingcap/kvproto#978, ref tikv#12362

This commit records read_index_propose_wait_nanos, read_index_confirm_wait_nanos and read_pool_schedule_wait_nanos

Signed-off-by: OneSizeFitQuorum <tanxinyu@apache.org>
Signed-off-by: fengou1 <feng.ou@pingcap.com>
ti-chi-bot pushed a commit that referenced this issue Sep 8, 2022
)

ref #12362

This commit adds `perform_read_local` fail_point so we can force the lease read to be triggered, which allows the test `test_read_execution_tracker` to pass stably.

Signed-off-by: OneSizeFitQuorum <tanxinyu@apache.org>
ti-chi-bot pushed a commit that referenced this issue Mar 23, 2023
ref #12362

Fix the returned apply write wal tracking time.

Signed-off-by: cfzjywxk <lsswxrxr@163.com>
ti-chi-bot added a commit that referenced this issue Mar 29, 2023
…heduler process duration (#14476)

ref #12362

Signed-off-by: cfzjywxk <lsswxrxr@163.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
ti-chi-bot bot pushed a commit that referenced this issue Jun 8, 2023
ref #12362

Signed-off-by: cfzjywxk <lsswxrxr@163.com>
tonyxuqqi pushed a commit to tonyxuqqi/tikv that referenced this issue Jun 22, 2023
ref tikv#12362

Signed-off-by: cfzjywxk <lsswxrxr@163.com>
Signed-off-by: tonyxuqqi <tonyxuqi@outlook.com>
ti-chi-bot bot pushed a commit that referenced this issue Dec 19, 2023
ref #12362

add log and metric in raft_client.

Signed-off-by: crazycs520 <crazycs520@gmail.com>
ti-chi-bot bot pushed a commit that referenced this issue Jun 12, 2024
…17070)

ref #12362

Add some necessary metrics for the observability on the duration of non-blocking and blocking ingestion of 
external sst files when scaling.

Meanwhile, it also introduces a new metric to observe the duration on the cost of clearing overlapped regions.

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

No branches or pull requests

2 participants