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

Trace client-side and server-side latency to diagnose RPC latency #11378

Open
sticnarf opened this issue Nov 16, 2021 · 8 comments
Open

Trace client-side and server-side latency to diagnose RPC latency #11378

sticnarf opened this issue Nov 16, 2021 · 8 comments
Labels
sig/diagnosis SIG: Diagnosis type/enhancement The issue or PR belongs to an enhancement.

Comments

@sticnarf
Copy link
Contributor

Feature Request

Time spent on RPC has always been the missing part of our metrics. There have been many cases when TiKV reports its process duration is short while the end user experiences high latency.

For example, TiDB has slow query logs and it is common for us to fail to find where the latency comes from. Sometimes, we blame the latency to the network and the RPC framework, without evidence.

If we record the client-side and server-side latency for each request, the client can easily know the time spent on the network and the RPC framework. This will help tease out the possible source of latency and guide us where to optimize. And it is vital to diagnose tail latency shown in slow query logs but not discoverable through metrics.

This can be implemented as part of full process tracing, but it seems #8981 has been inactive for months. But for this feature, a simpler implementation and protocol is enough.

@sticnarf sticnarf added type/enhancement The issue or PR belongs to an enhancement. sig/diagnosis SIG: Diagnosis labels Nov 16, 2021
@zhongzc
Copy link
Contributor

zhongzc commented Nov 18, 2021

But for this feature, a simpler implementation and protocol is enough.

Agreed. We can achieve this by recording more timestamps in the RPC process.

@sticnarf
Copy link
Contributor Author

sticnarf commented Nov 19, 2021

I implement it and test a TiDB cluster deployed on a single machine, and find the difference is larger than I expected...

The 99th difference can be up to two-digit milliseconds..

image

@cfzjywxk
Copy link
Collaborator

There are some related discussions in pingcap/tidb#28937.

@BusyJay
Copy link
Member

BusyJay commented Nov 24, 2021

Similar protocols can also be implemented for raft replications.

The latency should be split as send latency and receive latency. The sum is a RTT, and monitor them respectively may find potential pitfalls.

@sticnarf
Copy link
Contributor Author

The latency should be split as send latency and receive latency. The sum is a RTT, and monitor them respectively may find potential pitfalls.

Single trip latency is more difficult because we will be comparing two instants on different computers. It may depend on the precision of NTP.

@BusyJay
Copy link
Member

BusyJay commented Nov 25, 2021

Time is usually synced. We can find the difference if there are gaps.

@sticnarf
Copy link
Contributor Author

Time is usually synced. We can find the difference if there are gaps.

I worry the error may mislead us to wrong conclusions.

For example, if the clock of the receiver is 5ms slower (possible in a multi-region deployment) than the sender, we will find the send latency is shorter than the receive latency even if the actual latency is the same.

@BusyJay
Copy link
Member

BusyJay commented Nov 25, 2021

It seems time sync monitor is supported by node exporter, which can be used directly. https://github.com/prometheus/node_exporter/blob/master/docs/TIME.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/diagnosis SIG: Diagnosis type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

No branches or pull requests

4 participants