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

Configurable KV Timeout Tracking Issue #45380

Closed
9 tasks done
cfzjywxk opened this issue Jul 17, 2023 · 6 comments · Fixed by #45601
Closed
9 tasks done

Configurable KV Timeout Tracking Issue #45380

cfzjywxk opened this issue Jul 17, 2023 · 6 comments · Fixed by #45601
Labels
sig/transaction SIG:Transaction type/enhancement The issue or PR belongs to an enhancement.

Comments

@cfzjywxk
Copy link
Contributor

cfzjywxk commented Jul 17, 2023

Background

  • As described in the proposal, to mitigate the impact of latency jitter encountered by the tikv nodes.
  • The related feature request issue.

Tasks

TiDB

KV-Client

  • Add set interface SetKVReadTimeout and get interface GetKVReadTimeout for KVSnapshot. So the executors in TiDB could use them to get or set timeout values. Setting the tidb_kv_read_timeout in the Context.max_execution_duration_ms when get an batchGet are used. use tidb_kv_read_timeout as first kv request timeout tikv/client-go#919
  • Support specific timeout retry strategy in the KV-Client for the tidb_kv_read_timeout. When a timeout happens, retry the next available peer for the read requests like stale read and non-leader-only snapshot read requests. The strategy is:

TiKV

  • Support timeout check during the request handling in TiKV. When there's new point get and batch point get
    requests are received, the kv_read_timeout value should be read from the GetReuqest and BatchGetRequest
    requests and passed to the pointGetter. By now there's no canceling mechanism when the task is scheduled to the
    read pool in TiKV, a simpler way is to check the deadline duration before the next processing and try to return
    directly if the deadline is exceeded, but once the reading task is spawned to the read pool it could not be
    canceled anymore. storage: add deadline check in get and batch_get request handler tikv/tikv#15307
  • Support Timeout Configuration For Coprocessor Tasks. Use the kv_read_timeout value passed in to calculate the deadline result in parse_and_handle_unary_request instead of using the default one.
    • Coprocessor task already support kv_read_timeout.
@cfzjywxk cfzjywxk added type/enhancement The issue or PR belongs to an enhancement. sig/transaction SIG:Transaction labels Jul 17, 2023
@cfzjywxk
Copy link
Contributor Author

/cc @Tema @hihihuhu @crazycs520

@hihihuhu
Copy link
Contributor

i could first work on Add new session variable tidb_kv_read_timeout

@Tema
Copy link
Contributor

Tema commented Jul 25, 2023

@cfzjywxk can you create a dev branch for it where we can collaborate?

@cfzjywxk
Copy link
Contributor Author

@Tema
Copy link
Contributor

Tema commented Jul 27, 2023

@cfzjywxk could you please take a look at tikv/client-go#917. This will help us to work independently on tidb and client-go repos as we discussed.

@cfzjywxk
Copy link
Contributor Author

cfzjywxk commented Jul 27, 2023

@hihihuhu @Tema
I previously named the branch in a way that doesn't comply with CI standards, which caused CI triggers to malfunction. I have now renamed the branch to feature/dev_tidb_kv_read_timeout. Please take a look.

ti-chi-bot bot pushed a commit that referenced this issue Aug 11, 2023
crazycs520 pushed a commit to crazycs520/tidb that referenced this issue Aug 21, 2023
cfzjywxk pushed a commit that referenced this issue Aug 23, 2023
* kv request: configurable KV Timeout (#45601)

close #45380

* update go.mod

Signed-off-by: crazycs520 <[email protected]>

* refine code

Signed-off-by: crazycs520 <[email protected]>

* make bazel_prepare

Signed-off-by: crazycs520 <[email protected]>

* update go.mod

Signed-off-by: crazycs520 <[email protected]>

* update go.mod

Signed-off-by: crazycs520 <[email protected]>

---------

Signed-off-by: crazycs520 <[email protected]>
Co-authored-by: Chen Ding <[email protected]>
cfzjywxk pushed a commit that referenced this issue Aug 29, 2023
* kv request: configurable KV Timeout (#45601)

close #45380

* update go.mod

Signed-off-by: crazycs520 <[email protected]>

* refine code

Signed-off-by: crazycs520 <[email protected]>

* make bazel_prepare

Signed-off-by: crazycs520 <[email protected]>

* update go.mod

Signed-off-by: crazycs520 <[email protected]>

* update go.mod

Signed-off-by: crazycs520 <[email protected]>

---------

Signed-off-by: crazycs520 <[email protected]>
Co-authored-by: Chen Ding <[email protected]>
cfzjywxk pushed a commit that referenced this issue Sep 1, 2023
* kv request: configurable KV Timeout (#45601)

close #45380

* update go.mod

Signed-off-by: crazycs520 <[email protected]>

* refine code

Signed-off-by: crazycs520 <[email protected]>

* make bazel_prepare

Signed-off-by: crazycs520 <[email protected]>

* update go.mod

Signed-off-by: crazycs520 <[email protected]>

* update go.mod

Signed-off-by: crazycs520 <[email protected]>

---------

Signed-off-by: crazycs520 <[email protected]>
Co-authored-by: Chen Ding <[email protected]>
ti-chi-bot bot pushed a commit that referenced this issue Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/transaction SIG:Transaction type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants