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/06/01 23:47:47 UTC
[GitHub] [beam] kw2542 opened a new pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
kw2542 opened a new pull request #14923:
URL: https://github.com/apache/beam/pull/14923
1. Add JobServerTimeout option that defaults to 60s to all job server requests (with exception to the ones that are expected to last long or hang) to be on Par with python BEAM-7933
2. Add WaitForReady for ExternalEnvironment
**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).
`ValidatesRunner` compliance status (on master branch)
--------------------------------------------------------
<table>
<thead>
<tr>
<th>Lang</th>
<th>ULR</th>
<th>Dataflow</th>
<th>Flink</th>
<th>Samza</th>
<th>Spark</th>
<th>Twister2</th>
</tr>
</thead>
<tbody>
<tr>
<td>Go</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon">
</a>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
<tr>
<td>Java</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_ULR/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_ULR/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming/lastCompletedBuild/badge/icon?subject=V1+Streaming">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon?subject=V1+Java+11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2_Streaming/lastCompletedBuild/badge/icon?subject=V2+Streaming">
</a><br>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon?subject=Java+8">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon?subject=Java+11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon?subject=Portable+Streaming">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Samza/lastCompletedBuild/badge/icon?subject=Portable">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon?subject=Structured+Streaming">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon">
</a>
</td>
</tr>
<tr>
<td>Python</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon?subject=ValCont">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
<tr>
<td>XLang</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
</tbody>
</table>
Examples testing status on various runners
--------------------------------------------------------
<table>
<thead>
<tr>
<th>Lang</th>
<th>ULR</th>
<th>Dataflow</th>
<th>Flink</th>
<th>Samza</th>
<th>Spark</th>
<th>Twister2</th>
</tr>
</thead>
<tbody>
<tr>
<td>Go</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>Java</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Cron/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Java11_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Java11_Cron/lastCompletedBuild/badge/icon?subject=V1+Java11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_Examples_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_Examples_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>Python</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>XLang</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
</tbody>
</table>
Post-Commit SDK/Transform Integration Tests Status (on master branch)
------------------------------------------------------------------------------------------------
<table>
<thead>
<tr>
<th>Go</th>
<th>Java</th>
<th>Python</th>
</tr>
</thead>
<tbody>
<tr>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon?subject=3.6">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon?subject=3.7">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/badge/icon?subject=3.8">
</a>
</td>
</tr>
</tbody>
</table>
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
<table>
<thead>
<tr>
<th>---</th>
<th>Java</th>
<th>Python</th>
<th>Go</th>
<th>Website</th>
<th>Whitespace</th>
<th>Typescript</th>
</tr>
</thead>
<tbody>
<tr>
<td>Non-portable</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon">
</a><br>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon?subject=Tests">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon?subject=Lint">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/badge/icon?subject=Docker">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/badge/icon?subject=Docs">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
</tr>
<tr>
<td>Portable</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_GoPortable_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_GoPortable_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
</tbody>
</table>
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] kw2542 commented on a change in pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
kw2542 commented on a change in pull request #14923:
URL: https://github.com/apache/beam/pull/14923#discussion_r644185526
##########
File path: runners/java-fn-execution/src/main/java/org/apache/beam/runners/fnexecution/environment/ExternalEnvironmentFactory.java
##########
@@ -113,6 +113,7 @@ public RemoteEnvironment createEnvironment(Environment environment, String worke
ManagedChannelFactory.createDefault().forDescriptor(externalPayload.getEndpoint());
BeamFnApi.StartWorkerResponse startWorkerResponse =
BeamFnExternalWorkerPoolGrpc.newBlockingStub(managedChannel)
+ .withWaitForReady()
Review comment:
Good point, it should be separated out in a different PR at least.
My thought on this is similar as the job server request that when runner connects to external service, it is possible that external service may not be up and running yet if we bring up external service together with the main job. In addition, there may be intermittent errors, therefore, `withWaitForReady` seems to be a good candidate here.
I also think that we could provide customizable deadline/timeout here too which requires changes to external_payload model.
What are your thoughts on this?
--
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] kw2542 commented on pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
kw2542 commented on pull request #14923:
URL: https://github.com/apache/beam/pull/14923#issuecomment-853169758
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 commented on a change in pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
ibzib commented on a change in pull request #14923:
URL: https://github.com/apache/beam/pull/14923#discussion_r644194340
##########
File path: runners/java-fn-execution/src/main/java/org/apache/beam/runners/fnexecution/environment/ExternalEnvironmentFactory.java
##########
@@ -113,6 +113,7 @@ public RemoteEnvironment createEnvironment(Environment environment, String worke
ManagedChannelFactory.createDefault().forDescriptor(externalPayload.getEndpoint());
BeamFnApi.StartWorkerResponse startWorkerResponse =
BeamFnExternalWorkerPoolGrpc.newBlockingStub(managedChannel)
+ .withWaitForReady()
Review comment:
If I understand correctly, adding `withWaitForReady` will make requests that formerly failed fast instead block forever. Therefore, I don't think we should add `withWaitForReady` without also adding a timeout, since otherwise the user gets no feedback about why their pipeline is stalled. And I think the timeout should be off by default, since most deployments don't have a problem with starting the external service ahead of time.
--
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] kw2542 commented on pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
kw2542 commented on pull request #14923:
URL: https://github.com/apache/beam/pull/14923#issuecomment-853313112
--
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 a change in pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
ibzib commented on a change in pull request #14923:
URL: https://github.com/apache/beam/pull/14923#discussion_r644169234
##########
File path: runners/portability/java/src/main/java/org/apache/beam/runners/portability/PortableRunner.java
##########
@@ -201,12 +207,16 @@ public PipelineResult run(Pipeline pipeline) {
.setPreparationId(prepareJobResponse.getPreparationId())
.build();
- RunJobResponse runJobResponse = jobService.run(runJobRequest);
+ // Run the job and wait for a result, we don't set a timeout here because
+ // it may take a long time for a job to complete and streaming
+ // jobs currently never return a response.
Review comment:
Nit: wording
```suggestion
// jobs never return a response.
```
##########
File path: runners/portability/java/src/main/java/org/apache/beam/runners/portability/PortableRunner.java
##########
@@ -201,12 +207,16 @@ public PipelineResult run(Pipeline pipeline) {
.setPreparationId(prepareJobResponse.getPreparationId())
.build();
- RunJobResponse runJobResponse = jobService.run(runJobRequest);
+ // Run the job and wait for a result, we don't set a timeout here because
+ // it may take a long time for a job to complete and streaming
+ // jobs currently never return a response.
+ RunJobResponse runJobResponse = jobService.withDeadline(null).run(runJobRequest);
Review comment:
Why do we need to add `withDeadline(null)` here? Isn't it a no-op?
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/options/PortablePipelineOptions.java
##########
@@ -34,6 +34,17 @@
void setJobEndpoint(String endpoint);
+ @Description(
+ "Job service request timeout in seconds. The timeout "
+ + "determines the max time the driver program will wait to "
+ + "get a response from the job server. NOTE: the timeout does not "
+ + "apply to the actual pipeline run time. The driver program can "
Review comment:
Nit: wording
```suggestion
+ "apply to the actual pipeline run time. The driver program will "
```
##########
File path: runners/java-fn-execution/src/main/java/org/apache/beam/runners/fnexecution/environment/ExternalEnvironmentFactory.java
##########
@@ -113,6 +113,7 @@ public RemoteEnvironment createEnvironment(Environment environment, String worke
ManagedChannelFactory.createDefault().forDescriptor(externalPayload.getEndpoint());
BeamFnApi.StartWorkerResponse startWorkerResponse =
BeamFnExternalWorkerPoolGrpc.newBlockingStub(managedChannel)
+ .withWaitForReady()
Review comment:
What does `withWaitForReady` change, and why do we need it?
Also, I don't think this change belongs in the same PR as the job server changes. Where possible, PRs should do exactly one thing.
--
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 #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
ibzib commented on pull request #14923:
URL: https://github.com/apache/beam/pull/14923#issuecomment-854125819
Run Java Flink PortableValidatesRunner Batch
--
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] kw2542 commented on a change in pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
kw2542 commented on a change in pull request #14923:
URL: https://github.com/apache/beam/pull/14923#discussion_r644182569
##########
File path: runners/portability/java/src/main/java/org/apache/beam/runners/portability/PortableRunner.java
##########
@@ -201,12 +207,16 @@ public PipelineResult run(Pipeline pipeline) {
.setPreparationId(prepareJobResponse.getPreparationId())
.build();
- RunJobResponse runJobResponse = jobService.run(runJobRequest);
+ // Run the job and wait for a result, we don't set a timeout here because
+ // it may take a long time for a job to complete and streaming
+ // jobs currently never return a response.
+ RunJobResponse runJobResponse = jobService.withDeadline(null).run(runJobRequest);
Review comment:
You are right that we technically do not need it here because jobService was originally created without it. I added here because I wanted to explicit show that this call is made without any deadline.
I can remove it too, let me know which do you prefer.
--
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] kw2542 commented on pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
kw2542 commented on pull request #14923:
URL: https://github.com/apache/beam/pull/14923#issuecomment-859212106
--
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] kw2542 commented on a change in pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
kw2542 commented on a change in pull request #14923:
URL: https://github.com/apache/beam/pull/14923#discussion_r644195851
##########
File path: runners/java-fn-execution/src/main/java/org/apache/beam/runners/fnexecution/environment/ExternalEnvironmentFactory.java
##########
@@ -113,6 +113,7 @@ public RemoteEnvironment createEnvironment(Environment environment, String worke
ManagedChannelFactory.createDefault().forDescriptor(externalPayload.getEndpoint());
BeamFnApi.StartWorkerResponse startWorkerResponse =
BeamFnExternalWorkerPoolGrpc.newBlockingStub(managedChannel)
+ .withWaitForReady()
Review comment:
Removed this change in the PR, I will follow up separately on this.
--
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 #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
ibzib commented on pull request #14923:
URL: https://github.com/apache/beam/pull/14923#issuecomment-854125819
Run Java Flink PortableValidatesRunner Batch
--
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] apilloud commented on pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
apilloud commented on pull request #14923:
URL: https://github.com/apache/beam/pull/14923#issuecomment-859191488
Looks like this change broke the portable spark runner: https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/4428/
Are you aware of the breakage? Can we roll this back?
--
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 merged pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
ibzib merged pull request #14923:
URL: https://github.com/apache/beam/pull/14923
--
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] kw2542 commented on pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
kw2542 commented on pull request #14923:
URL: https://github.com/apache/beam/pull/14923#issuecomment-853405694
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] kw2542 commented on pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
kw2542 commented on pull request #14923:
URL: https://github.com/apache/beam/pull/14923#issuecomment-853405694
--
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] kw2542 commented on pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
kw2542 commented on pull request #14923:
URL: https://github.com/apache/beam/pull/14923#issuecomment-852567794
@ibzib Could you help review 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] kw2542 commented on pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
kw2542 commented on pull request #14923:
URL: https://github.com/apache/beam/pull/14923#issuecomment-852605927
Run Python_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] kw2542 commented on pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
kw2542 commented on pull request #14923:
URL: https://github.com/apache/beam/pull/14923#issuecomment-852567573
Let me know if you think we should add timeout in external_payload 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] ibzib merged pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
ibzib merged pull request #14923:
URL: https://github.com/apache/beam/pull/14923
--
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 a change in pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
ibzib commented on a change in pull request #14923:
URL: https://github.com/apache/beam/pull/14923#discussion_r644188136
##########
File path: runners/portability/java/src/main/java/org/apache/beam/runners/portability/PortableRunner.java
##########
@@ -201,12 +207,16 @@ public PipelineResult run(Pipeline pipeline) {
.setPreparationId(prepareJobResponse.getPreparationId())
.build();
- RunJobResponse runJobResponse = jobService.run(runJobRequest);
+ // Run the job and wait for a result, we don't set a timeout here because
+ // it may take a long time for a job to complete and streaming
+ // jobs currently never return a response.
+ RunJobResponse runJobResponse = jobService.withDeadline(null).run(runJobRequest);
Review comment:
I prefer removing it. The comment is sufficient to show our intent here.
--
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] kw2542 commented on pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
kw2542 commented on pull request #14923:
URL: https://github.com/apache/beam/pull/14923#issuecomment-853960139
@ibzib Could you help take another pass?
--
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] kw2542 commented on pull request #14923: [BEAM-12419] Add Timeout and WaitForReady support in java gRPC calls
Posted by GitBox <gi...@apache.org>.
kw2542 commented on pull request #14923:
URL: https://github.com/apache/beam/pull/14923#issuecomment-852620368
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