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: fix bindinfo bugs when update cache #13875

Merged
merged 2 commits into from
Dec 4, 2019
Merged

Conversation

alivxxx
Copy link
Contributor

@alivxxx alivxxx commented Dec 3, 2019

What problem does this PR solve?

  1. Fix wrong behavior for SQL bind when table is dropped #13825
  2. When there are multiply bindings in one bucket of cache, we cannot delete them after call of removeDeletedBindRecord because the result is not stored back.

What is changed and how it works?

  1. Read bindinfo from last update time, but not include it.
  2. Store the result back to cache after update.

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)
    Use the exact step as in the issue and there are no repeated error.

Code changes

  • Has exported function/method change

Side effects

-None

Related changes

  • Need to cherry-pick to the release branch

Release note

  • Fix repeated error log and multiply same bindings in global cache when update the bindinfo cache

@alivxxx alivxxx requested a review from a team as a code owner December 3, 2019 09:36
@ghost ghost requested review from eurekaka and francis0407 and removed request for a team December 3, 2019 09:36
@alivxxx alivxxx requested a review from winoros December 3, 2019 09:37
@codecov
Copy link

codecov bot commented Dec 3, 2019

Codecov Report

Merging #13875 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #13875   +/-   ##
===========================================
  Coverage   80.2224%   80.2224%           
===========================================
  Files           479        479           
  Lines        118781     118781           
===========================================
  Hits          95289      95289           
  Misses        15970      15970           
  Partials       7522       7522

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

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

@lzmhhh123 lzmhhh123 added status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. labels Dec 4, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Dec 4, 2019

Your auto merge job has been accepted, waiting for 13820

@sre-bot
Copy link
Contributor

sre-bot commented Dec 4, 2019

/run-all-tests

@sre-bot sre-bot merged commit e9d1142 into pingcap:master Dec 4, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Dec 4, 2019

cherry pick to release-3.0 failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wrong behavior for SQL bind when table is dropped
5 participants