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

drainer: fix codec race condition in integration test #16845

Merged
merged 1 commit into from
Apr 11, 2023

Conversation

tgross
Copy link
Member

@tgross tgross commented Apr 11, 2023

While working on #14348 I encountered corruption in the RPC decoding in the drainer integration test.

msgpackrpc codec handles are specific to a connection and cannot be shared between goroutines; this can cause corrupted decoding. Fix the drainer integration test so that we create separate codecs for the goroutines that the test helper spins up to simulate client updates.

This changeset also refactors the drainer integration test to bring it up to current idioms and library usages, make assertions more clear, and reduce duplication. I'm backporting this work for consistency between release branches but there's no changelog because this is a test-only PR.

msgpackrpc codec handles are specific to a connection and cannot be shared
between goroutines; this can cause corrupted decoding. Fix the drainer
integration test so that we create separate codecs for the goroutines that the
test helper spins up to simulate client updates.

This changeset also refactors the drainer integration test to bring it up to
current idioms and library usages, make assertions more clear, and reduce
duplication.
@tgross tgross added theme/drain theme/testing Test related issues backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line labels Apr 11, 2023
@tgross tgross requested review from shoenig and gulducat April 11, 2023 15:14
@tgross tgross marked this pull request as ready for review April 11, 2023 15:15
Copy link
Member

@shoenig shoenig left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line theme/drain theme/testing Test related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants