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 2022/11/16 00:49:51 UTC
[GitHub] [beam] apilloud opened a new pull request, #24188: Handle CompleteWorkStatus shutdown signal
apilloud opened a new pull request, #24188:
URL: https://github.com/apache/beam/pull/24188
This fixes crashes caused by ignoring the field.
**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`).
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] 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/get-started-contributing/#make-the-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
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)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
apilloud commented on PR #24188:
URL: https://github.com/apache/beam/pull/24188#issuecomment-1319177591
R: @kennknowles
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
apilloud commented on PR #24188:
URL: https://github.com/apache/beam/pull/24188#issuecomment-1320394942
Build failure is #19465
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
apilloud commented on PR #24188:
URL: https://github.com/apache/beam/pull/24188#issuecomment-1322460081
R: @kileys
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
apilloud commented on PR #24188:
URL: https://github.com/apache/beam/pull/24188#issuecomment-1320397197
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud merged pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
apilloud merged PR #24188:
URL: https://github.com/apache/beam/pull/24188
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24188:
URL: https://github.com/apache/beam/pull/24188#issuecomment-1316149473
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on a diff in pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
apilloud commented on code in PR #24188:
URL: https://github.com/apache/beam/pull/24188#discussion_r1025796697
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/WorkItemStatusClient.java:
##########
@@ -116,6 +117,10 @@ public synchronized void setWorker(
/** Return the {@link WorkItemServiceState} resulting from sending an error completion status. */
public synchronized WorkItemServiceState reportError(Throwable e) throws IOException {
checkState(!finalStateSent, "cannot reportUpdates after sending a final state");
+ if (wasAskedToAbort) {
+ LOG.info("Service already asked to abort work item, not reporting ignored progress.");
+ return null;
Review Comment:
Added.
This method has a `return execute(status)` and `execute` returns `@Nullable WorkItemServiceState`. There are also tests that return null, which I discovered with a previous variant of this PR that didn't handle that case. Any Java Object can be null.
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
apilloud commented on PR #24188:
URL: https://github.com/apache/beam/pull/24188#issuecomment-1320471475
Filed #24263.
@kennknowles tests are passing. Please take a look.
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a diff in pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
kennknowles commented on code in PR #24188:
URL: https://github.com/apache/beam/pull/24188#discussion_r1025781798
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/WorkItemStatusClient.java:
##########
@@ -116,6 +117,10 @@ public synchronized void setWorker(
/** Return the {@link WorkItemServiceState} resulting from sending an error completion status. */
public synchronized WorkItemServiceState reportError(Throwable e) throws IOException {
checkState(!finalStateSent, "cannot reportUpdates after sending a final state");
+ if (wasAskedToAbort) {
+ LOG.info("Service already asked to abort work item, not reporting ignored progress.");
+ return null;
Review Comment:
Add `@Nullable` to the return type. Are you sure it is safe? I don't see other nulls returned. I guess checking is turned off for this class...
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
apilloud commented on PR #24188:
URL: https://github.com/apache/beam/pull/24188#issuecomment-1319319121
Run Java_Examples_Dataflow 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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
apilloud commented on PR #24188:
URL: https://github.com/apache/beam/pull/24188#issuecomment-1317686366
R: @lukecwik
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a diff in pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
kennknowles commented on code in PR #24188:
URL: https://github.com/apache/beam/pull/24188#discussion_r1034021724
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/WorkItemStatusClient.java:
##########
@@ -116,6 +117,10 @@ public synchronized void setWorker(
/** Return the {@link WorkItemServiceState} resulting from sending an error completion status. */
public synchronized WorkItemServiceState reportError(Throwable e) throws IOException {
checkState(!finalStateSent, "cannot reportUpdates after sending a final state");
+ if (wasAskedToAbort) {
+ LOG.info("Service already asked to abort work item, not reporting ignored progress.");
+ return null;
Review Comment:
Thanks for adding it. This will be one less for the person who eventually enables checkerframework.
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
apilloud commented on PR #24188:
URL: https://github.com/apache/beam/pull/24188#issuecomment-1319160935
assign set of reviewers
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
apilloud commented on PR #24188:
URL: https://github.com/apache/beam/pull/24188#issuecomment-1319318910
Build failure is #20819
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
apilloud commented on PR #24188:
URL: https://github.com/apache/beam/pull/24188#issuecomment-1320395066
Run Java_Examples_Dataflow 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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
apilloud commented on PR #24188:
URL: https://github.com/apache/beam/pull/24188#issuecomment-1319291552
If modern type checking was turned on I would not be able to `return null;` without getting a warning about missing `@Nullable` annotations. I removed the "@SuppressWarnings("nullness")` tag from this class with the hope that it will just work, but I am not volunteering to fix unrelated problems so and will add it back if there are additional breakages.
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
kennknowles commented on PR #24188:
URL: https://github.com/apache/beam/pull/24188#issuecomment-1319287359
Within our own code, with modern type checking turned on, things cannot be null unless they are declared. We've had major problems due to not adhering to this discipline.
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
apilloud commented on PR #24188:
URL: https://github.com/apache/beam/pull/24188#issuecomment-1319298670
Unfortunately the type checker found 9 additional issues, so I have turned it back off.
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #24188: Handle CompleteWorkStatus shutdown signal
Posted by GitBox <gi...@apache.org>.
apilloud commented on PR #24188:
URL: https://github.com/apache/beam/pull/24188#issuecomment-1320397092
The precommit failure doesn't have an open bug https://ci-beam.apache.org/job/beam_PreCommit_Java_Commit/24868/console
FATAL: command execution failed
15:35:24 hudson.remoting.ChannelClosedException: Channel "hudson.remoting.Channel@6e74f455:apache-beam-jenkins-3": Remote call on apache-beam-jenkins-3 failed. The channel is closing down or has closed down
15:35:24 at hudson.remoting.Channel.call(Channel.java:993)
15:35:24 at hudson.remoting.RemoteInvocationHandler.invoke(RemoteInvocationHandler.java:285)
15:35:24 at com.sun.proxy.$Proxy140.isAlive(Unknown Source)
15:35:24 at hudson.Launcher$RemoteLauncher$ProcImpl.isAlive(Launcher.java:1215)
15:35:24 at hudson.Launcher$RemoteLauncher$ProcImpl.join(Launcher.java:1207)
15:35:24 at hudson.Launcher$ProcStarter.join(Launcher.java:524)
15:35:24 at hudson.plugins.gradle.Gradle.perform(Gradle.java:317)
15:35:24 at hudson.tasks.BuildStepMonitor$1.perform(BuildStepMonitor.java:20)
15:35:24 at hudson.model.AbstractBuild$AbstractBuildExecution.perform(AbstractBuild.java:814)
15:35:24 at hudson.model.Build$BuildExecution.build(Build.java:199)
15:35:24 at hudson.model.Build$BuildExecution.doRun(Build.java:164)
15:35:24 at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:522)
15:35:24 at hudson.model.Run.execute(Run.java:1896)
15:35:24 at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:44)
15:35:24 at hudson.model.ResourceController.execute(ResourceController.java:101)
15:35:24 at hudson.model.Executor.run(Executor.java:442)
15:35:24 Caused by: java.io.IOException: Pipe closed after 0 cycles
15:35:24 at org.apache.sshd.common.channel.ChannelPipedInputStream.read(ChannelPipedInputStream.java:126)
15:35:24 at org.apache.sshd.common.channel.ChannelPipedInputStream.read(ChannelPipedInputStream.java:105)
15:35:24 at hudson.remoting.FlightRecorderInputStream.read(FlightRecorderInputStream.java:94)
15:35:24 at hudson.remoting.ChunkedInputStream.readHeader(ChunkedInputStream.java:75)
15:35:24 at hudson.remoting.ChunkedInputStream.readUntilBreak(ChunkedInputStream.java:105)
15:35:24 at hudson.remoting.ChunkedCommandTransport.readBlock(ChunkedCommandTransport.java:39)
15:35:24 at hudson.remoting.AbstractSynchronousByteArrayCommandTransport.read(AbstractSynchronousByteArrayCommandTransport.java:34)
15:35:24 at hudson.remoting.SynchronousCommandTransport$ReaderThread.run(SynchronousCommandTransport.java:61)
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org