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: fix the duplicate binding case when updating bind info #22367

Merged
merged 3 commits into from
Jan 12, 2021

Conversation

rebelice
Copy link
Contributor

What problem does this PR solve?

Issue Number: close #22309

Problem Summary:

This PR will fix the duplicate binding case when updating bind info

New update method used.
We will read all bindings in the memory first. Then remove the duplicate bindings and keep only the latest. Finally, insert bindings after restoring with DB.

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test

Side effects

  • Performance regression
    • Consumes more CPU
    • Consumes more MEM
  • Breaking backward compatibility

Release note

  • session: fix the duplicate binding case when updating bind info

@rebelice
Copy link
Contributor Author

/label needs-cherry-pick-4.0, sig/planner, needs-cherry-pick-5.0-rc

@rebelice
Copy link
Contributor Author

/cc @qw4990, @eurekaka

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 the status/LGT1 Indicates that a PR has LGTM 1. label Jan 12, 2021
@rebelice
Copy link
Contributor Author

/rebuild

@github-actions github-actions bot added the sig/sql-infra SIG: SQL Infra label Jan 12, 2021
Copy link
Contributor

@qw4990 qw4990 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 Jan 12, 2021
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Jan 12, 2021
@qw4990
Copy link
Contributor

qw4990 commented Jan 12, 2021

/merge

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

/run-all-tests

@ti-srebot ti-srebot merged commit dd234b7 into pingcap:master Jan 12, 2021
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Jan 12, 2021
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #22368

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Jan 12, 2021
@ti-srebot
Copy link
Contributor

cherry pick to release-5.0-rc in PR #22369

ti-srebot added a commit that referenced this pull request Jan 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic/sql-plan-management sig/planner SIG: Planner sig/sql-infra SIG: SQL Infra 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.

The bug of restoring SQL bindings when upgrading the version
4 participants