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

changefeedccl: fix kafka messagetoolarge test failure #98471

Merged
merged 1 commit into from
Mar 13, 2023

Conversation

samiskin
Copy link
Contributor

Fixes: #93847

This fixes the following bug in the TestChangefeedKafkaMessageTooLarge test setup:

  1. The feed starts sending messages, randomly triggering a MessageTooLarge error causing a retry with a smaller batch size
  2. Eventually, while the retrying process is still ongoing, all 2000 rows are successfully received by the mock kafka sink, causing assertPayloads to complete, causing the test to closeFeed and run CANCEL on the changefeed.
  3. The retrying process gets stuck in sendMessage where it can't send the message to the feedCh which has been closed since the changefeed is trying to close, but it also can't exit on the mock sink's tg.done since that only closes after the feed fully closes, which requires the retrying process to end.

Release note: None

Fixes: cockroachdb#93847

This fixes the following bug in the TestChangefeedKafkaMessageTooLarge
test setup:
1. The feed starts sending messages, randomly triggering a
   MessageTooLarge error causing a retry with a smaller batch size
2. Eventually, while the retrying process is still ongoing, all 2000
   rows are successfully received by the mock kafka sink, causing
   assertPayloads to complete, causing the test to closeFeed and run
   CANCEL on the changefeed.
3. The retrying process gets stuck in sendMessage where it can't send
   the message to the feedCh which has been closed since the changefeed
   is trying to close, but it also can't exit on the mock sink's tg.done
   since that only closes after the feed fully closes, which requires
   the retrying process to end.

Release note: None
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@samiskin samiskin marked this pull request as ready for review March 13, 2023 13:08
@samiskin samiskin requested a review from a team as a code owner March 13, 2023 13:08
@samiskin samiskin requested review from miretskiy and removed request for a team March 13, 2023 13:08
@samiskin
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Mar 13, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Mar 13, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Mar 13, 2023

Build succeeded:

@craig craig bot merged commit c31c1ac into cockroachdb:master Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ccl/changefeedccl: TestChangefeedKafkaMessageTooLarge failed
3 participants