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

Better handle empty control batches #1256

Merged
merged 9 commits into from
Feb 7, 2022

Conversation

julienvincent
Copy link
Contributor

@julienvincent julienvincent commented Dec 19, 2021

This is a two part fix for transactionally aware consumers that are processing aborted transactions.

Instrumentation Events

The problem with the current implementation is that if a batch containing only control records or only aborted messages is received then there is no way for monitors to be aware that the consumer progressed. This can happen, for example, when reading aborted transactions.

This is fixed by emitting start and end batch instrumentation events if the batch is an empty control batch or only contains aborted messages.

Consume Lockout

Secondarily there exists an issue wherein if a batch containing only aborted messages or only control messages is received then the consumer never resolves + commits the batch offset, resulting in the consumer processing the same batch repeatedly and getting blocked.

Similarly to the previous scenario, this is fixed by checking for this scenario and resolving + auto-committing the batches latest offset.

@julienvincent julienvincent changed the title Emit instrumentation events for empty control batches Better handle empty control batches Dec 19, 2021
@julienvincent julienvincent force-pushed the fix/batch-monitoring branch 2 times, most recently from 6523ea5 to 0480ed5 Compare December 19, 2021 21:00
@julienvincent
Copy link
Contributor Author

@Nevon happy to merge?

@julienvincent
Copy link
Contributor Author

@Nevon Looks like the pipeline had an ephemeral network failure. Can you re-trigger the jobs?

@Nevon
Copy link
Collaborator

Nevon commented Jan 10, 2022

Triggered another run now. Not sure if the Azure DevOps Github integration is smart enough for the build status notifications to work. If not, you can always push an empty commit to trigger it.

@julienvincent
Copy link
Contributor Author

@Nevon Ok, I re-pushed to force the jobs. All is passing 👍

@julienvincent
Copy link
Contributor Author

Hey @Nevon, any chance I could get some eyes on this?

@Nevon
Copy link
Collaborator

Nevon commented Feb 2, 2022

Next week I will have some time to spend on KafkaJS, and will make sure that this is high up on the todo list.

@julienvincent
Copy link
Contributor Author

Thanks! 🙏

@Nevon Nevon merged commit d0288b3 into tulios:master Feb 7, 2022
@julienvincent julienvincent deleted the fix/batch-monitoring branch February 7, 2022 08:17
@Nevon Nevon mentioned this pull request Feb 7, 2022
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.

2 participants