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

tikv: fix panic after sending fail with ambiguous errors #17211

Merged
merged 3 commits into from
May 25, 2020
Merged

tikv: fix panic after sending fail with ambiguous errors #17211

merged 3 commits into from
May 25, 2020

Conversation

lysu
Copy link
Contributor

@lysu lysu commented May 14, 2020

What problem does this PR solve?

Issue Number: close #17201

Problem Summary:

batch client cleans up request map after sending fail, but tikv have the chance to receive requests when some ambiguous error happened.

and we should get panic when tikv make responses to those ambiguous requests now

What is changed and how it works?

Proposal: xxx

What's Changed:

log instead of panic

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Integration test(need take more log in tipocket)

Side effects

  • n/a

Release note

  • Fix panic after sending fail with ambiguous errors

This change is Reviewable

@lysu lysu requested a review from hicqu May 14, 2020 10:02
@lysu
Copy link
Contributor Author

lysu commented May 14, 2020

@hicqu PTAL, and maybe need help to recheck tikv's impl

@lysu
Copy link
Contributor Author

lysu commented May 14, 2020

ref tikv/tikv#7324 and #15943 - -

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@lysu lysu removed the status/DNM label May 25, 2020
@lysu
Copy link
Contributor Author

lysu commented May 25, 2020

@jackysp need a approve

@jackysp
Copy link
Member

jackysp commented May 25, 2020

/merge

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

sre-bot commented May 25, 2020

/run-all-tests

@codecov
Copy link

codecov bot commented May 25, 2020

Codecov Report

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

@@             Coverage Diff             @@
##             master     #17211   +/-   ##
===========================================
  Coverage   79.8423%   79.8423%           
===========================================
  Files           520        520           
  Lines        139897     139897           
===========================================
  Hits         111697     111697           
  Misses        19236      19236           
  Partials       8964       8964           

@sre-bot
Copy link
Contributor

sre-bot commented May 25, 2020

cherry pick to release-3.0 in PR #17378

@sre-bot
Copy link
Contributor

sre-bot commented May 25, 2020

cherry pick to release-3.1 in PR #17379

@sre-bot
Copy link
Contributor

sre-bot commented May 25, 2020

cherry pick to release-4.0 in PR #17380

jebter added a commit that referenced this pull request May 25, 2020
…7380)

Co-authored-by: lysu <sulifx@gmail.com>
Co-authored-by: jebter <jebter@126.com>
sre-bot added a commit that referenced this pull request Jun 8, 2020
…7378)

* tikv: fix panic after sending fail with ambiguous errors (#17211)

* fix compile error

Co-authored-by: lysu <sulifx@gmail.com>
sre-bot added a commit that referenced this pull request Jun 8, 2020
…7379)

* tikv: fix panic after sending fail with ambiguous errors (#17211)

* fix compile

Co-authored-by: lysu <sulifx@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/tikv status/can-merge Indicates a PR has been approved by a committer. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Couldn't find request when recv response and panic in batchclient recvloop
4 participants