You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2021/03/03 22:31:43 UTC
[GitHub] [beam] Fokko opened a new pull request #14139: BEAM-11920: Simplify the pipeline by grouping on TableDestination
Fokko opened a new pull request #14139:
URL: https://github.com/apache/beam/pull/14139
Currently we group the TableDestinations and the related field on a surrogate key, which will run everything sequentially. Instead we can also group on the TableDestination, simplifying the pipeline while maintaining a bit more parallelism, creating the temp tables in parallel.
**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:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
--- | --- | --- | --- | --- | --- | ---
Go | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam
.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website | Whitespace | Typescript
--- | --- | --- | --- | --- | --- | ---
Non-portable | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) <br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/)
Portable | --- | [](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | --- | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #14139: BEAM-11920: Simplify the pipeline by grouping on TableDestination
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #14139:
URL: https://github.com/apache/beam/pull/14139#issuecomment-796316850
I guess for streaming, the pipeline would infinitely retry the failed keys, and succeed on others, so perhaps it makes sense to get this change in.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on pull request #14139: BEAM-11920: Simplify the pipeline by grouping on TableDestination
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on pull request #14139:
URL: https://github.com/apache/beam/pull/14139#issuecomment-796914129
@Fokko I don't think this will reduce the size of the serialized graph much, if at all.
I'm looking to see how to modify your fix to fix the bug I mentioned. I'll update if I have ideas.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #14139: BEAM-11920: Simplify the pipeline by grouping on TableDestination
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #14139:
URL: https://github.com/apache/beam/pull/14139#issuecomment-796231173
ah yes thanks! I am a little concerned about this. The reason we group everything is that we want to make sure all loads succeed before scheduling any copy jobs. this makes sense for batch (stage n+1 won't execute before stage n is completely finished) - but I worry about streaming - is it possible for keys from a single destination to be processed by n+1 before all keys in stage N are finalized?
@reuvenlax is this possible?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] Fokko closed pull request #14139: BEAM-11920: Simplify the pipeline by grouping on TableDestination
Posted by GitBox <gi...@apache.org>.
Fokko closed pull request #14139:
URL: https://github.com/apache/beam/pull/14139
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] Fokko commented on pull request #14139: BEAM-11920: Simplify the pipeline by grouping on TableDestination
Posted by GitBox <gi...@apache.org>.
Fokko commented on pull request #14139:
URL: https://github.com/apache/beam/pull/14139#issuecomment-796657180
@reuvenlax thanks for looking at this.
Allow me to elaborate a bit. I'm running into limits of the size of the graph, and noticed that the writing to bigquery consists of many steps. My aim was to reduce these a bit, I've noticed that the surrogate key wasn't required, and therefore this patch. We have a streaming job that reads from many queue's, and writes this data to bigquery. Reducing the number of steps, makes the graph smaller, and allows us to run pipelines within a single job.
Instead of combining the elements in a single operator, using the ArrayListMultimap, I think it is aesthetically nicer to use the Beam primitives and combine on an actual key instead of a synthetic one.
I fully agree that we should not break the update compatibility if not strictly required, but it would be great if we can combine this with #14139. I've cherry-picked the commit into my branch.
I'll run this into our test environment.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] Fokko commented on pull request #14139: BEAM-11920: Simplify the pipeline by grouping on TableDestination
Posted by GitBox <gi...@apache.org>.
Fokko commented on pull request #14139:
URL: https://github.com/apache/beam/pull/14139#issuecomment-812139361
@pabloem I would suggest closing this for now. Did some tests, and don't see any improvements :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on pull request #14139: BEAM-11920: Simplify the pipeline by grouping on TableDestination
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on pull request #14139:
URL: https://github.com/apache/beam/pull/14139#issuecomment-796437016
First comments
- This change isn't likely to improve any performance. Temp tables are only generated if the BQ load job is loading more than 11 terabytes of data, otherwise the data is simply loaded directly into BQ from files. This means that the vast majority of BQ loads never generate temp tables, and even if a single job was loading 100TB, it would only generate 10 temp table.
- For streaming, this change will break update compatibility for the Dataflow runner. We usually try to avoid this, and the minor savings here probably doesn't justify this. That being said, there's another PR - pr/12968 - that fixes a critical bug that might also break update compatibility, so if we can't figure out how to fix that bug in a compatible way, we might have a window to make changes.
Looking at pr/12968, I think it actually misses fixing the pane-index bug for the WriteRename path, and your change here is a step towards fixing that (though the actual fix I think is a bit more complicated than what you have here). I suggest that we merge these two PRs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #14139: BEAM-11920: Simplify the pipeline by grouping on TableDestination
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #14139:
URL: https://github.com/apache/beam/pull/14139#issuecomment-791028843
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #14139: BEAM-11920: Simplify the pipeline by grouping on TableDestination
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #14139:
URL: https://github.com/apache/beam/pull/14139#issuecomment-811472621
@reuvenlax what should we do about this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] Fokko commented on pull request #14139: BEAM-11920: Simplify the pipeline by grouping on TableDestination
Posted by GitBox <gi...@apache.org>.
Fokko commented on pull request #14139:
URL: https://github.com/apache/beam/pull/14139#issuecomment-796221647
@pabloem Do you have time to look at this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #14139: BEAM-11920: Simplify the pipeline by grouping on TableDestination
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #14139:
URL: https://github.com/apache/beam/pull/14139#issuecomment-796349873
Run Java PostCommit
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on pull request #14139: BEAM-11920: Simplify the pipeline by grouping on TableDestination
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on pull request #14139:
URL: https://github.com/apache/beam/pull/14139#issuecomment-796353095
I'm taking a look. Also would like to make sure this doesn't affect update.
On Wed, Mar 10, 2021 at 5:22 PM Pablo ***@***.***> wrote:
> Run Java PostCommit
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/beam/pull/14139#issuecomment-796349873>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AFAYJVLL6GVKCHVCUQWOM2TTDAEK3ANCNFSM4YSC5MHA>
> .
>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org