-
Notifications
You must be signed in to change notification settings - Fork 4.5k
[Bug] Fix PubsubUnboundedSink not following 1000 messages per batch limit #37178
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
base: master
Are you sure you want to change the base?
[Bug] Fix PubsubUnboundedSink not following 1000 messages per batch limit #37178
Conversation
Summary of ChangesHello @userzhy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
This commit fixes the issue where PubsubUnboundedSink's WriterFn was not properly enforcing the message count limit per publish batch. The publishBatchSize parameter was being passed to the constructor but not stored or used in the processElement method. Changes: - Add publishBatchSize field to WriterFn class - Store publishBatchSize in both WriterFn constructors - Add message count check in processElement alongside existing byte size check - Update both PubsubSinkDynamicDestinations and PubsubSink to pass publishBatchSize when creating WriterFn instances The fix ensures that batches are split when they reach either the message count limit or the byte size limit, preventing Pubsub from rejecting batches that exceed the 1000 messages per request limit. Fixes apache#36885
325f313 to
f70b0f2
Compare
|
Assigning reviewers: R: @chamikaramj for label java. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Fix PubsubUnboundedSink's WriterFn not enforcing message count limit per publish batch
Description
This PR fixes a bug where
PubsubUnboundedSink'sWriterFnwas not properly enforcing the message count limit per publish batch. ThepublishBatchSizeparameter was being passed to the constructor but was never stored or used in theprocessElementmethod.According to Google Cloud Pub/Sub resource limits, a single publish request can contain at most 1000 messages. Without this fix, batches could exceed this limit, causing Pub/Sub to reject the publish request.
Changes
publishBatchSizefield toWriterFnclasspublishBatchSizein bothWriterFnconstructorsprocessElementalongside existing byte size checkPubsubSinkDynamicDestinationsandPubsubSinkto passpublishBatchSizewhen creatingWriterFninstancesTesting
PubsubUnboundedSinkTesttests passsendMoreThanOneBatchByNumMessagesspecifically validates batch splitting by message countFixes #36885
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.