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

*: not send tso request when point get with max tso (#11981) #16336

Merged
merged 4 commits into from
Apr 14, 2020

Conversation

sre-bot
Copy link
Contributor

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

cherry-pick #11981 to release-3.1


What problem does this PR solve?

Point get without double read will use maxUint64 as transaction tso. Then no need to send tso request to PD.

This PR will delay to get txn future after try fast point, so it may have a little performance regression in the case of need to get tso from PD. Especially in the situation of point get with double read.

BenchMark

Environment: In my local machine: 1 TiDB, 1 PD, 5 TiKV.

Best case: Point get with max tso

Before

  • point get QPS is 25.7k

image

image

This PR

  • point get QPS is 31.4k

image

image

Worst case: Point get with double read can't use max tso

Before

  • point get with double read QPS is 17.8k

In this PR

  • point get with double read QPS is 17.1k

As you can see, since I delay use get txn future, there is a performance regression is this situation.

What is changed and how it works?

Check List

Tests

  • No test.

Code changes

  • Has exported function/method change

@sre-bot
Copy link
Contributor Author

sre-bot commented Apr 13, 2020

/run-all-tests

@jackysp
Copy link
Member

jackysp commented Apr 14, 2020

/run-all-tests

@coocood
Copy link
Member

coocood commented Apr 14, 2020

LGTM

1 similar comment
@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao tiancaiamao added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 14, 2020
@tiancaiamao tiancaiamao added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Apr 14, 2020
@jackysp
Copy link
Member

jackysp commented Apr 14, 2020

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 14, 2020
@sre-bot
Copy link
Contributor Author

sre-bot commented Apr 14, 2020

/run-all-tests

@sre-bot
Copy link
Contributor Author

sre-bot commented Apr 14, 2020

@sre-bot merge failed.

@zz-jason
Copy link
Member

/run-sqllogic-test-2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution sig/planner SIG: Planner sig/transaction SIG:Transaction status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug. type/enhancement The issue or PR belongs to an enhancement. type/performance type/3.1-cherry-pick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants