Try to fix race again - don't reuse clients for sender/receiver #971
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@embano1 I ran the tests several times via the "checks" and they seem happy. I did see some weirdness in the mqtt stuff but we can look at that separately - haven't seen a kafka complaint yet. Thoughts? |
embano1
reviewed
Oct 27, 2023
| topicName := "test-ce-client-" + uuid.New().String() | ||
| p, err := kafka_sarama.NewProtocolFromClient(client, topicName, topicName, kafka_sarama.WithReceiverGroupId(TEST_GROUP_ID)) | ||
|
|
||
| clientR := testClient(t) |
There was a problem hiding this comment.
nit: please add a code comment or better variable name to indicate what the R and S suffixes on client and p mean so our future-selves have an easier time understanding the change :)
There was a problem hiding this comment.
Done - I can barely remember why/what I did yesterday so this is a good "nit" :-)
Signed-off-by: Doug Davis <[email protected]>
embano1
approved these changes
Oct 28, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
from: https://pkg.go.dev/github.com/shopify/sarama#NewConsumerGroupFromClient
NewConsumerGroupFromClient creates a new consumer group using the given client. It is still necessary to call Close() on the underlying client when shutting down this consumer. PLEASE NOTE: consumer groups can only re-use but not share clients.
I'm not 100% sure if using the same client for sending and receiving qualifies for this restriction or not but let's see if this works w/o the "sleeps". Seems to work locally for me.
So, this PR just creates 2 clients (one for sending and one for receiving) instead of trying to reuse the same one for both.