gemini-code-assist[bot] commented on code in PR #38894:
URL: https://github.com/apache/beam/pull/38894#discussion_r3389291552


##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/util/common/worker/ReadOperation.java:
##########
@@ -47,7 +48,7 @@
   "nullness" // TODO(https://github.com/apache/beam/issues/20497)
 })
 public class ReadOperation extends Operation {
-  public static class ReadLoopAbortedException extends InterruptedException {
+  public static class ReadLoopAbortedException extends InterruptedIOException {

Review Comment:
   ![high](https://www.gstatic.com/codereviewagent/high-priority.svg)
   
   Changing `ReadLoopAbortedException` to extend `InterruptedIOException` 
instead of `InterruptedException` prevents catch blocks targeting 
`InterruptedException` from catching this exception. Any orchestration or 
execution logic (such as in `MapTaskExecutor`) that previously relied on 
catching `InterruptedException` to handle logical aborts must be updated to 
handle `InterruptedIOException` or `ReadLoopAbortedException` directly to avoid 
unhandled exception propagation or incorrect error state transitions.



##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowBatchWorkerHarness.java:
##########
@@ -92,6 +92,7 @@ public void run() throws InterruptedException {
 
     worker.startStatusServer();
     processWork(pipelineOptions, worker, Sleeper.DEFAULT);
+    throw new RuntimeException("All worker threads have died. Exiting.");

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   Throwing an unconditional `RuntimeException` when `processWork` returns will 
cause the JVM to exit with a non-zero status even during a graceful shutdown 
(such as during clean termination or in testing environments). If `processWork` 
can exit normally under clean shutdown conditions, this exception should only 
be thrown if the shutdown was unexpected or if the worker threads actually 
failed.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to