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 2020/06/25 01:29:28 UTC
[GitHub] [beam] ihji opened a new pull request #12086: [BEAM-10322] allow only single assignment to producing stages by pcol…
ihji opened a new pull request #12086:
URL: https://github.com/apache/beam/pull/12086
…lection map
------------------------
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
--- | --- | --- | --- | --- | ---
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_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/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/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_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35_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_Flink/lastCompletedBuild/) | --- | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
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/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_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.
----------------------------------------------------------------
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] ihji commented on pull request #12086: [BEAM-10322] allow only single assignment to producing stages by pcol…
Posted by GitBox <gi...@apache.org>.
ihji commented on pull request #12086:
URL: https://github.com/apache/beam/pull/12086#issuecomment-649161049
R: @pablom
CC: @robertwb
----------------------------------------------------------------
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 #12086: [BEAM-10322] allow only single assignment to producing stages by pcol…
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #12086:
URL: https://github.com/apache/beam/pull/12086#issuecomment-652228492
can you share how to reproduce the issue, Heejong? @ihji
----------------------------------------------------------------
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 #12086: [BEAM-10322] allow only single assignment to producing stages by pcol…
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #12086:
URL: https://github.com/apache/beam/pull/12086#issuecomment-652206508
Run Python2_PVR_Flink PreCommit
----------------------------------------------------------------
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] aaltay commented on pull request #12086: [BEAM-10322] allow only single assignment to producing stages by pcol…
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #12086:
URL: https://github.com/apache/beam/pull/12086#issuecomment-654982821
Folks just checking. I do not see an LGTM or an approval on this PR. Is everyone OK with it being merged?
----------------------------------------------------------------
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] robertwb commented on a change in pull request #12086: [BEAM-10322] allow only single assignment to producing stages by pcol…
Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #12086:
URL: https://github.com/apache/beam/pull/12086#discussion_r449317183
##########
File path: sdks/python/apache_beam/runners/portability/fn_api_runner/execution.py
##########
@@ -375,6 +375,8 @@ def get_all_side_inputs():
for o in transform.outputs.values():
if o in s.side_inputs():
continue
+ if o in producing_stages_by_pcoll:
Review comment:
Why is it OK to ignore duplicates?
----------------------------------------------------------------
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] robertwb commented on pull request #12086: [BEAM-10322] allow only single assignment to producing stages by pcol…
Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #12086:
URL: https://github.com/apache/beam/pull/12086#issuecomment-653267148
Could you clarify what the bug that you're trying to fix here is?
----------------------------------------------------------------
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 #12086: [BEAM-10322] allow only single assignment to producing stages by pcol…
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #12086:
URL: https://github.com/apache/beam/pull/12086#issuecomment-653371609
We're trying to find producer stages of a PCollection. The first stage
encountered is the producer (due to the topological order) Subsequent
stages encountered with subtransforms producing the PCollection are
consumers where a Data input transform reads in the PCollection.
I can go back to add the comment.
On Thu, Jul 2, 2020, 5:08 PM Robert Bradshaw <no...@github.com>
wrote:
> *@robertwb* commented on this pull request.
> ------------------------------
>
> In sdks/python/apache_beam/runners/portability/fn_api_runner/execution.py
> <https://github.com/apache/beam/pull/12086#discussion_r449317183>:
>
> > @@ -375,6 +375,8 @@ def get_all_side_inputs():
> for o in transform.outputs.values():
> if o in s.side_inputs():
> continue
> + if o in producing_stages_by_pcoll:
>
> Why is it OK to ignore duplicates?
>
> —
> You are receiving this because you modified the open/close state.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/beam/pull/12086#pullrequestreview-442079350>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAJ5Z3HA54C75UNJTXSLX3TRZUOORANCNFSM4OHTZ6JQ>
> .
>
----------------------------------------------------------------
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 merged pull request #12086: [BEAM-10322] allow only single assignment to producing stages by pcol…
Posted by GitBox <gi...@apache.org>.
pabloem merged pull request #12086:
URL: https://github.com/apache/beam/pull/12086
----------------------------------------------------------------
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] ihji commented on pull request #12086: [BEAM-10322] allow only single assignment to producing stages by pcol…
Posted by GitBox <gi...@apache.org>.
ihji commented on pull request #12086:
URL: https://github.com/apache/beam/pull/12086#issuecomment-653377848
Here's a concrete example that can be fixed by this PR:
```
Stage A:
(Input PCollection i - PTransform 1 - Output PCollection j)
Stage B:
(Input PCollection j - PTransform 2 - Output PCollection k)
Stage C:
(Input PCollection j for side input - PTransform 3)
```
We want to find `Stage A` for emitting side input for `Stage C`. However, some synthetic PTransforms are inserted during pipeline optimization phase:
```
Stage A:
(Input PCollection i - PTransform 1 - Output PCollection j)
(Input PCollection j - Synthetic Write - Data Sink)
Stage B:
(Data source - Synthetic Read - Output PCollection j)
(Input PCollection j - PTransform 2 - Output PCollection k)
Stage C:
(Input PCollection j for side input - PTransform 3)
```
If we allow multiple assignments to `producing_stages_by_pcoll`, `Stage B` will emit side input for `Stage C` (topologically `Stage B` comes after `Stage A`). Since `Stage B` and `Stage C` have no dependencies, the pipeline will succeed when `Stage B` is executed first and fail when `Stage C` is executed before `Stage B`.
----------------------------------------------------------------
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 #12086: [BEAM-10322] allow only single assignment to producing stages by pcol…
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #12086:
URL: https://github.com/apache/beam/pull/12086#issuecomment-655006468
My bad. Heejong and I discussed it via chat last week. I forgot to add the
LGTM when I merged.
On Tue, Jul 7, 2020, 9:36 AM Ahmet Altay <no...@github.com> wrote:
> Folks just checking. I do not see an LGTM or an approval on this PR. Is
> everyone OK with it being merged?
>
> —
> You are receiving this because you modified the open/close state.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/beam/pull/12086#issuecomment-654982821>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAJ5Z3ASYENCBI7LTG4RQMDR2NFI7ANCNFSM4OHTZ6JQ>
> .
>
----------------------------------------------------------------
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