-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
feat: Always pass a commit policy to Arroyo stream processor #3257
Conversation
Soon this will become a mandatory argument
Codecov ReportBase: 92.30% // Head: 92.33% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #3257 +/- ##
==========================================
+ Coverage 92.30% 92.33% +0.03%
==========================================
Files 697 697
Lines 31630 31611 -19
==========================================
- Hits 29195 29187 -8
+ Misses 2435 2424 -11
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
tick_consumer, | ||
Topic(commit_log_topic.topic_name), | ||
factory, | ||
tick_consumer, Topic(commit_log_topic.topic_name), factory, ONCE_PER_SECOND |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, why is this one ONCE_PER_SECOND
? Was that defined somewhere else already? Based on the other changes I assumed IMMEDIATE
was the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, this is actually changing from IMMEDIATE to ONCE_PER_SECOND. This is because committing every offset to Kafka is a bad idea and we should actually throttle here. It's the only functional change in this PR. The others do batch processing already and commit once at the end of the batch so I left them all as IMMEDIATE.
Soon this will become a mandatory argument
Soon this will become a mandatory argument