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

kv-client: fix stream is not recycled when meeting recv error (#1336) #1373

Merged
merged 2 commits into from
Jan 28, 2021

Conversation

ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Jan 28, 2021

cherry-pick #1336 to release-4.0
You can switch your code base to this Pull Request by using git-extras:

# In ticdc repo:
git pr https://github.com/pingcap/ticdc/pull/1373

After apply modifications, you can push your change to this PR via:

git push git@github.com:ti-srebot/ticdc.git pr/1373:release-4.0-96fb4549fff4

What problem does this PR solve?

  • Find a but that gRPC stream is not recycled correctly in some scenarios
    • if stream.Recv with error, but the underlying stream is normal actually (or the next round of stream.Send is successfully, then the receiveFromStream goroutine will be never created, and corresponding events will not be consumed by kv client.
  • Improve unit test of kv client, part of Improve unit tests of kv client #1292

What is changed and how it works?

  • Close stream before receiveFromStream routine exits, and remove stream from stream cache
  • Add unit test to cover this scenario

Check List

Tests

  • Unit test
  • Integration test

Release note

  • Fix a bug that kv client may skip to recreate event feed receiving routine by accident.

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

/run-all-tests

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 28, 2021
@amyangfei
Copy link
Contributor

/merge

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

Your auto merge job has been accepted, waiting for:

  • 1370

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot merged commit 602f251 into pingcap:release-4.0 Jan 28, 2021
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/LGT1 Indicates that a PR has LGTM 1. status/ptal Could you please take a look? type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants