client/tso: use the TSO request pool at the tsoClient level to avoid data race #8077

JmPotato · 2024-04-16T09:18:56Z

What problem does this PR solve?

Issue Number: close #8055, ref #8076.

What is changed and how does it work?

Use the TSO request pool at the `tsoClient` level to avoid data race.

Check List

Tests

Unit test
Integration test

Release note

None.

ti-chi-bot · 2024-04-16T09:18:58Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

nolouch
rleungx

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

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

codecov · 2024-04-16T12:55:13Z

Codecov Report

Merging #8077 (2cec6b5) into master (2dd2301) will decrease coverage by 0.04%.
Report is 1 commits behind head on master.
The diff coverage is 90.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8077      +/-   ##
==========================================
- Coverage   77.29%   77.26%   -0.04%     
==========================================
  Files         469      470       +1     
  Lines       61097    61104       +7     
==========================================
- Hits        47225    47212      -13     
- Misses      10313    10321       +8     
- Partials     3559     3571      +12

Flag	Coverage Δ
unittests	`77.26% <90.00%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

rleungx · 2024-04-17T02:26:09Z

client/client.go

@@ -838,12 +823,22 @@ func (c *client) dispatchTSORequestWithRetry(req *tsoRequest) error {
 			err = errs.ErrClientGetTSO.FastGenByArgs("tso client is nil")
 			continue
 		}
+		// Get a new request from the pool if it's nil or not from the current pool.
+		if req == nil || req.pool != tsoClient.tsoReqPool {


When will req.pool != tsoClient.tsoReqPool happen?

During the retry period, a mode switch occurred causing the tsoClient to be updated.

JmPotato · 2024-04-17T13:23:52Z

/merge

ti-chi-bot · 2024-04-17T13:23:56Z

@JmPotato: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2024-04-17T13:23:58Z

This pull request has been accepted and is ready to merge.

Commit hash: 43b4611

ti-chi-bot · 2024-04-30T07:05:46Z

In response to a cherrypick label: new pull request created to branch release-8.1: #8141.

ti-chi-bot · 2024-04-30T07:06:22Z

In response to a cherrypick label: new pull request could not be created: failed to create pull request against tikv/pd#release-8.1 from head ti-chi-bot:cherry-pick-8077-to-release-8.1: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for ti-chi-bot:cherry-pick-8077-to-release-8.1."}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request"}

…data race (#8077) (#8141) close #8055, ref #8076 Use the TSO request pool at the `tsoClient` level to avoid data race. Signed-off-by: JmPotato <[email protected]> Co-authored-by: JmPotato <[email protected]>

JmPotato added component/client Client logic. component/tso Timestamp Oracle. labels Apr 16, 2024

JmPotato requested review from rleungx and HuSharp April 16, 2024 09:18

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 16, 2024

JmPotato marked this pull request as draft April 16, 2024 09:56

ti-chi-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 16, 2024

JmPotato added 2 commits April 16, 2024 20:34

Use the TSO request pool at the tsoClient level to avoid data race

fa9c534

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

Adapt dispatch retry logic

43b4611

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

JmPotato force-pushed the fix_tso_pool_race branch from 7f5f1e7 to 43b4611 Compare April 16, 2024 12:34

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 16, 2024

JmPotato marked this pull request as ready for review April 16, 2024 12:48

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 16, 2024

rleungx reviewed Apr 17, 2024

View reviewed changes

rleungx approved these changes Apr 17, 2024

View reviewed changes

ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 17, 2024

nolouch approved these changes Apr 17, 2024

View reviewed changes

ti-chi-bot bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Apr 17, 2024

ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 17, 2024

Merge branch 'master' into fix_tso_pool_race

2cec6b5

ti-chi-bot bot merged commit db88a43 into tikv:master Apr 17, 2024
26 checks passed

JmPotato deleted the fix_tso_pool_race branch April 18, 2024 02:29

ti-chi-bot added the needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. label Apr 30, 2024

JmPotato removed the needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. label Apr 30, 2024

ti-chi-bot added the needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. label Apr 30, 2024

ti-chi-bot mentioned this pull request Apr 30, 2024

client/tso: use the TSO request pool at the tsoClient level to avoid data race (#8077) #8141

Merged

JmPotato mentioned this pull request Apr 30, 2024

Update the PD client to adopt the latest optimization tikv/client-go#1326

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

client/tso: use the TSO request pool at the tsoClient level to avoid data race #8077

client/tso: use the TSO request pool at the tsoClient level to avoid data race #8077

JmPotato commented Apr 16, 2024

ti-chi-bot bot commented Apr 16, 2024 •

edited

Loading

codecov bot commented Apr 16, 2024 •

edited

Loading

rleungx Apr 17, 2024

JmPotato Apr 17, 2024

JmPotato commented Apr 17, 2024

ti-chi-bot bot commented Apr 17, 2024

ti-chi-bot bot commented Apr 17, 2024

ti-chi-bot commented Apr 30, 2024

ti-chi-bot commented Apr 30, 2024

client/tso: use the TSO request pool at the tsoClient level to avoid data race #8077

client/tso: use the TSO request pool at the tsoClient level to avoid data race #8077

Conversation

JmPotato commented Apr 16, 2024

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot bot commented Apr 16, 2024 • edited Loading

codecov bot commented Apr 16, 2024 • edited Loading

Codecov Report

rleungx Apr 17, 2024

Choose a reason for hiding this comment

JmPotato Apr 17, 2024

Choose a reason for hiding this comment

JmPotato commented Apr 17, 2024

ti-chi-bot bot commented Apr 17, 2024

ti-chi-bot bot commented Apr 17, 2024

ti-chi-bot commented Apr 30, 2024

ti-chi-bot commented Apr 30, 2024

ti-chi-bot bot commented Apr 16, 2024 •

edited

Loading

codecov bot commented Apr 16, 2024 •

edited

Loading