Skip to content

changefeedccl: fix kafka messagetoolarge test failure#98471

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
samiskin:kafka-message-too-large-test-fix
Mar 13, 2023
Merged

changefeedccl: fix kafka messagetoolarge test failure#98471
craig[bot] merged 1 commit intocockroachdb:masterfrom
samiskin:kafka-message-too-large-test-fix

Conversation

@samiskin
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Mar 13, 2023

Build failed (retrying...):

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Mar 13, 2023

Build failed (retrying...):

@craig
Copy link
Copy Markdown
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