Coalesce sources until compressed serialized bundles under API limit #26267
+98
−8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Verified that the unit test fails in current master and suceeds with fix in. Current master: With the fix: |
|
R: @chamikaramj @kennknowles per file history internal tracker: 277995484 |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
liferoad
reviewed
Apr 13, 2023
| + "of %d bytes is still larger than the limit %d. For more information, please " | ||
| + "check the corresponding FAQ entry at " | ||
| + "https://cloud.google.com/dataflow/pipelines/troubleshooting-your-pipeline", | ||
| source, bundles.size(), serializedSize, apiByteLimit); |
There was a problem hiding this comment.
Shall we suggest this workaround with withTemplateCompatiblity?
There was a problem hiding this comment.
This option is BigQueryIO.read only while the bug is generic to dataflow worker. With that option the read teansform will expand differently and likely avoided the problematic code path here (haven't verify)
There was a problem hiding this comment.
The workaround will be using Runner v2. That will convert sources into SDFs that do not run into this source split limit. We should suggest that here.
There was a problem hiding this comment.
Hi @chamikaramj is this code path only for Dataflow runner v1? If so we could document the recommendation of runner v2 also in https://cloud.google.com/dataflow/docs/guides/common-errors#boundedsource-objects-splitintobundles
There was a problem hiding this comment.
Yeah, this codepath (source split operation) is only get hit by Runner v1. +1 for documenting in the Website.
liferoad
reviewed
Apr 13, 2023
| String.format( | ||
| "Unable to coalesce the sources into compressed serialized bundles to satisfy the " | ||
| + "allowable limit when splitting %s. With %d bundles, total serialized size " | ||
| + "of %d bytes is still larger than the limit %d. For more information, please " |
There was a problem hiding this comment.
can we use this https://cloud.google.com/dataflow/docs/guides/common-errors#boundedsource-objects-splitintobundles? Please update the previous LOG.info with this link as well.
There was a problem hiding this comment.
Good catch, this url is more accurate
liferoad
approved these changes
Apr 13, 2023
|
Run Java PreCommit |
liferoad
approved these changes
Apr 17, 2023
|
LGTM |
|
Thanks for suggestions. The linked cloud documentation is going to be updated. Merging the fix for now. |
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #26264
The original
limitNumberOfBundleslogic has problems that it always limits to 100 bundles if the split bundle is greater than this number and did not check the resulting response size. It could result in IllegalArgumentException immediately below.Please add a meaningful description for your change here
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.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.