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

session, tikv: allocate task IDs for distsql requests (#16520) #17155

Merged
merged 4 commits into from
May 14, 2020

Conversation

sre-bot
Copy link
Contributor

@sre-bot sre-bot commented May 13, 2020

cherry-pick #16520 to release-4.0


What problem does this PR solve?

Issue Number: close tikv/tikv#7484

Problem Summary:

Currently TiDB provides no information for TiKV that some group of requests belong to the same statement (task). So TiKV's unified read pool uses the start TS of the transaction as the task identifier . It's not suitable in all cases to use start ts as the task id for the unified read pool. For example, in the same transaction, a big query is executed and the following point selects are also downgraded.

What is changed and how it works?

What's Changed:

Currently in this PR, we only set task ids for distsql requests that can be run in transactions. For other distsql requests (like DDL reorg, analyze and checksum), the task id is not provided and TiKV still uses start_ts as task id. And TiKV uses random numbers as the task ids for point gets commands so this PR also does not set task ids for these requests.

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test

Side effects

Release note

Allocate task IDs for distsql requests to help TiKV schedule them better.

@sre-bot sre-bot requested a review from a team as a code owner May 13, 2020 03:48
@sre-bot
Copy link
Contributor Author

sre-bot commented May 13, 2020

/run-all-tests

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sticnarf please resolve conflict and fix CI.

@sticnarf
Copy link
Contributor

/run-unit-tests

@sticnarf
Copy link
Contributor

/run-unit-tests

@sticnarf sticnarf requested a review from zz-jason May 13, 2020 06:46
@sticnarf sticnarf modified the milestones: 4.0.0-rc.2, v4.0.0-ga May 13, 2020
@sticnarf sticnarf requested a review from XuHuaiyu May 13, 2020 07:24
Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zz-jason zz-jason added status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. labels May 14, 2020
@sre-bot
Copy link
Contributor Author

sre-bot commented May 14, 2020

Your auto merge job has been accepted, waiting for:

  • 17118
  • 17107
  • 16165
  • 17003
  • 17126
  • 17164
  • 17167

@sre-bot
Copy link
Contributor Author

sre-bot commented May 14, 2020

/run-all-tests

@sre-bot sre-bot merged commit f71cdf8 into pingcap:release-4.0 May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/session component/tikv sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/4.0-cherry-pick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants