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/04/14 17:57:19 UTC
[GitHub] [beam] lukecwik opened a new pull request #11414: [BEAM-5605,
BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
lukecwik opened a new pull request #11414: [BEAM-5605, BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
URL: https://github.com/apache/beam/pull/11414
The next step is to plumb the split request to the BeamFnDataReadRunner which will forward it to the FnApiDoFnRunner.
------------------------
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 | Apex | Dataflow | Flink | Gearpump | Samza | Spark
--- | --- | --- | --- | --- | --- | --- | ---
Go | [](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
Java | [](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
Python | [](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
XLang | --- | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://builds.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
With regards,
Apache Git Services
[GitHub] [beam] boyuanzz commented on a change in pull request #11414:
[BEAM-5605, BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
Posted by GitBox <gi...@apache.org>.
boyuanzz commented on a change in pull request #11414: [BEAM-5605, BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
URL: https://github.com/apache/beam/pull/11414#discussion_r408482142
##########
File path: sdks/java/harness/src/main/java/org/apache/beam/fn/harness/FnApiDoFnRunner.java
##########
@@ -761,72 +795,111 @@ public void processElementForElementAndRestriction(
continue;
}
- // Make sure to get the output watermark before we split to ensure that the lower bound
- // applies to both the primary and residual.
- KV<Instant, WatermarkEstimatorStateT> watermarkAndState =
- currentWatermarkEstimator.getWatermarkAndState();
- SplitResult<RestrictionT> result = currentTracker.trySplit(0);
+ // Attempt to checkpoint the current restriction.
+ HandlesSplits.SplitResult splitResult =
Review comment:
Why do we want to checkpoint after processElement finishing?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] lukecwik merged pull request #11414: [BEAM-5605,
BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
Posted by GitBox <gi...@apache.org>.
lukecwik merged pull request #11414: [BEAM-5605, BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
URL: https://github.com/apache/beam/pull/11414
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11414: [BEAM-5605,
BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11414: [BEAM-5605, BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
URL: https://github.com/apache/beam/pull/11414#issuecomment-614099306
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
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on a change in pull request #11414:
[BEAM-5605, BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11414: [BEAM-5605, BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
URL: https://github.com/apache/beam/pull/11414#discussion_r408487984
##########
File path: sdks/java/harness/src/main/java/org/apache/beam/fn/harness/FnApiDoFnRunner.java
##########
@@ -761,72 +795,111 @@ public void processElementForElementAndRestriction(
continue;
}
- // Make sure to get the output watermark before we split to ensure that the lower bound
- // applies to both the primary and residual.
- KV<Instant, WatermarkEstimatorStateT> watermarkAndState =
- currentWatermarkEstimator.getWatermarkAndState();
- SplitResult<RestrictionT> result = currentTracker.trySplit(0);
+ // Attempt to checkpoint the current restriction.
+ HandlesSplits.SplitResult splitResult =
Review comment:
The DoFn returned `ProcessContinuation#resume` and not `ProcessContinuation#stop`.
The current restriction within the restriction tracker represents the entire restriction so splitting it provides the remainder and should update the restriction tracker so that the primary is in a done state so that `checkDone` down below can succeed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] boyuanzz commented on a change in pull request #11414:
[BEAM-5605, BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
Posted by GitBox <gi...@apache.org>.
boyuanzz commented on a change in pull request #11414: [BEAM-5605, BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
URL: https://github.com/apache/beam/pull/11414#discussion_r409204369
##########
File path: sdks/java/harness/src/main/java/org/apache/beam/fn/harness/FnApiDoFnRunner.java
##########
@@ -761,72 +795,111 @@ public void processElementForElementAndRestriction(
continue;
}
- // Make sure to get the output watermark before we split to ensure that the lower bound
- // applies to both the primary and residual.
- KV<Instant, WatermarkEstimatorStateT> watermarkAndState =
- currentWatermarkEstimator.getWatermarkAndState();
- SplitResult<RestrictionT> result = currentTracker.trySplit(0);
+ // Attempt to checkpoint the current restriction.
+ HandlesSplits.SplitResult splitResult =
Review comment:
Got it. Thanks!
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11414: [BEAM-5605,
BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11414: [BEAM-5605, BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
URL: https://github.com/apache/beam/pull/11414#issuecomment-614934489
Run RAT 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
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11414: [BEAM-5605,
BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11414: [BEAM-5605, BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
URL: https://github.com/apache/beam/pull/11414#issuecomment-613593412
CC: @youngoli
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11414: [BEAM-5605,
BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11414: [BEAM-5605, BEAM-2939] Add support for FnApiDoFnRunner to handle split calls.
URL: https://github.com/apache/beam/pull/11414#issuecomment-613590197
R: @ihji @boyuanzz
----------------------------------------------------------------
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
With regards,
Apache Git Services