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

bindinfo: add debugging to plan evolve (#20523) #20530

Merged
merged 1 commit into from
Oct 20, 2020

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #20523 to release-4.0


What problem does this PR solve?

Issue Number: Part of #20417

Problem Summary:

Currently it is very difficult to debug why a new plan is rejected. I believe there is a current bug in the logic, but independently, I've added debugging information and added a threshold of 2x on current execution time just so the error log can contain more stats on the new plan.

What is changed and how it works?

What's Changed:

Rather than timing out a new plan if it won't meet the new query time threshold, an independent threshold is set that is actually longer than the current execution time. This is helpful for debugging purposes.

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Manual tested to see error written to the log when plan was rejected.

Side effects

  • More time spent on a plan that will ultimately be rejected.

Release note

  • When verifying potential new plans, plan binding will now wait before timing out worse plans so that more debug information can be written to the TiDB error log.

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor Author

/run-all-tests

Copy link
Contributor

@lzmhhh123 lzmhhh123 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 20, 2020
Copy link
Contributor

@eurekaka eurekaka 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 status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 20, 2020
@eurekaka eurekaka merged commit b1d20a2 into pingcap:release-4.0 Oct 20, 2020
@eurekaka eurekaka deleted the release-4.0-e5a9e4690fb4 branch October 20, 2020 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/planner SIG: Planner 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.

3 participants