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

planner, config: Enable plan cache by default #20416

Merged
merged 6 commits into from Oct 29, 2020
Merged

planner, config: Enable plan cache by default #20416

merged 6 commits into from Oct 29, 2020

Conversation

ghost
Copy link

@ghost ghost commented Oct 12, 2020

What problem does this PR solve?

Fixes #20306

Problem Summary:

Prepared Plan Cache has been validated by QA as stable, and starting with TiDB 5.0 is safe to be enabled by default. This PR still offers the toggle to disable the cache (which is technically obsolete, since you could set the capacity of the cache to zero as well). I think this makes sense for now because several other stable features still have an 'enable' flag, but I can adjust if reviewers would like me to take another look.

What is changed and how it works?

Proposal: xxx

What's Changed:

How it Works:

Related changes

Check List

Tests

  • Unit test

Side effects

  • Query plans or behaviors may change for some queries.

Release note

  • The prepare plan cache is now enabled by default. This helps improve performance when using prepared statements.

@ghost ghost self-requested a review as a code owner October 12, 2020 19:03
@ghost ghost requested review from XuHuaiyu and removed request for a team October 12, 2020 19:03
@ghost ghost mentioned this pull request Oct 12, 2020
9 tasks
@ghost ghost requested a review from SunRunAway October 12, 2020 19:05
@ghost ghost self-requested a review as a code owner October 12, 2020 19:47
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

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 13, 2020
Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Oct 13, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Oct 13, 2020
@SunRunAway
Copy link
Contributor

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Oct 13, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@nullnotnil merge failed.

@ghost
Copy link
Author

ghost commented Oct 13, 2020

/run-all-tests

@zz-jason
Copy link
Member

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit e102c12 into pingcap:master Oct 29, 2020
@ghost ghost deleted the prepared-stmt-on-by-default branch October 29, 2020 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/config component/expression sig/execution SIG execution sig/planner SIG: Planner status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable Prepared Statement cache by default
3 participants