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/08/27 23:18:34 UTC
[GitHub] [beam] lukecwik opened a new pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
lukecwik opened a new pull request #12708:
URL: https://github.com/apache/beam/pull/12708
------------------------
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_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://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/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/)<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_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_Direct/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
--- | --- | --- | --- | --- | ---
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/)
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)
------------------------------------------------------------------------------------------------
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] ibzib commented on pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
ibzib commented on pull request #12708:
URL: https://github.com/apache/beam/pull/12708#issuecomment-699167629
I suspect this caused Flink postcommit to fail: https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/5393/
cc: @robinyqiu
----------------------------------------------------------------
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] lukecwik edited a comment on pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik edited a comment on pull request #12708:
URL: https://github.com/apache/beam/pull/12708#issuecomment-682318772
R: @mxm @tweise
I fixed the timeout issue, please take a look at #12706 first before reviewing this as this includes the commit from that PR as well.
----------------------------------------------------------------
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] lukecwik commented on pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #12708:
URL: https://github.com/apache/beam/pull/12708#issuecomment-682238832
Note that #12706 should be reviewed and merged first.
----------------------------------------------------------------
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] lukecwik commented on pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #12708:
URL: https://github.com/apache/beam/pull/12708#issuecomment-682318882
Run Flink ValidatesRunner
----------------------------------------------------------------
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] lukecwik merged pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik merged pull request #12708:
URL: https://github.com/apache/beam/pull/12708
----------------------------------------------------------------
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] ibzib edited a comment on pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
ibzib edited a comment on pull request #12708:
URL: https://github.com/apache/beam/pull/12708#issuecomment-699167629
I suspect this caused portable Flink postcommit to fail: BEAM-10974, https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/5393/
cc: @robinyqiu
----------------------------------------------------------------
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] mxm commented on a change in pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #12708:
URL: https://github.com/apache/beam/pull/12708#discussion_r479392671
##########
File path: runners/flink/src/main/java/org/apache/beam/runners/flink/FlinkStreamingPipelineTranslator.java
##########
@@ -99,6 +99,13 @@ public CompositeBehavior enterCompositeTransform(TransformHierarchy.Node node) {
PTransform<?, ?> transform = node.getTransform();
if (transform != null) {
+ // TODO(BEAM-10670): Remove this and the ReadTranslator once the "use_deprecated_read"
+ // experiment is removed. Don't translate composite Read transforms since we expect the
+ // primitive expansion containing an SDF to be used.
+ if (PTransformTranslation.READ_TRANSFORM_URN.equals(
+ PTransformTranslation.urnForTransformOrNull(transform))) {
+ return CompositeBehavior.ENTER_TRANSFORM;
+ }
Review comment:
Makes sense!
----------------------------------------------------------------
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] lukecwik merged pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik merged pull request #12708:
URL: https://github.com/apache/beam/pull/12708
----------------------------------------------------------------
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] ibzib edited a comment on pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
ibzib edited a comment on pull request #12708:
URL: https://github.com/apache/beam/pull/12708#issuecomment-699167629
I suspect this caused portable Flink postcommit to fail: https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/5393/
cc: @robinyqiu
----------------------------------------------------------------
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] lukecwik commented on pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #12708:
URL: https://github.com/apache/beam/pull/12708#issuecomment-682318772
R: @mxm @tweise
I fixed the timeout issue, please take a look at #12706 first.
----------------------------------------------------------------
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] lukecwik commented on pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #12708:
URL: https://github.com/apache/beam/pull/12708#issuecomment-682801887
Run Java 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] ibzib edited a comment on pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
ibzib edited a comment on pull request #12708:
URL: https://github.com/apache/beam/pull/12708#issuecomment-699167629
I suspect this caused portable Flink postcommit to fail: BEAM-10964, https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/5393/
cc: @robinyqiu
----------------------------------------------------------------
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] lukecwik commented on a change in pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #12708:
URL: https://github.com/apache/beam/pull/12708#discussion_r479375163
##########
File path: runners/flink/src/main/java/org/apache/beam/runners/flink/FlinkStreamingPipelineTranslator.java
##########
@@ -99,6 +99,13 @@ public CompositeBehavior enterCompositeTransform(TransformHierarchy.Node node) {
PTransform<?, ?> transform = node.getTransform();
if (transform != null) {
+ // TODO(BEAM-10670): Remove this and the ReadTranslator once the "use_deprecated_read"
+ // experiment is removed. Don't translate composite Read transforms since we expect the
+ // primitive expansion containing an SDF to be used.
+ if (PTransformTranslation.READ_TRANSFORM_URN.equals(
+ PTransformTranslation.urnForTransformOrNull(transform))) {
+ return CompositeBehavior.ENTER_TRANSFORM;
+ }
Review comment:
The non SDF version of Read is always a primitive transform so it gets replaced down below in `visitPrimitiveTransform`.
----------------------------------------------------------------
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] lukecwik merged pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik merged pull request #12708:
URL: https://github.com/apache/beam/pull/12708
----------------------------------------------------------------
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] lukecwik commented on pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #12708:
URL: https://github.com/apache/beam/pull/12708#issuecomment-682238857
Run Flink ValidatesRunner
----------------------------------------------------------------
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] mxm commented on a change in pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #12708:
URL: https://github.com/apache/beam/pull/12708#discussion_r478919501
##########
File path: runners/flink/src/main/java/org/apache/beam/runners/flink/FlinkStreamingPipelineTranslator.java
##########
@@ -99,6 +99,13 @@ public CompositeBehavior enterCompositeTransform(TransformHierarchy.Node node) {
PTransform<?, ?> transform = node.getTransform();
if (transform != null) {
+ // TODO(BEAM-10670): Remove this and the ReadTranslator once the "use_deprecated_read"
+ // experiment is removed. Don't translate composite Read transforms since we expect the
+ // primitive expansion containing an SDF to be used.
+ if (PTransformTranslation.READ_TRANSFORM_URN.equals(
+ PTransformTranslation.urnForTransformOrNull(transform))) {
+ return CompositeBehavior.ENTER_TRANSFORM;
+ }
Review comment:
How will this work with the opt-out of SDF? Will primitive Reads still be translated then?
----------------------------------------------------------------
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