LEEKYE commented on code in PR #35120:
URL: https://github.com/apache/beam/pull/35120#discussion_r2220313156
##########
sdks/java/harness/src/main/java/org/apache/beam/fn/harness/FnHarness.java:
##########
@@ -279,7 +279,15 @@ public static void main(
ExecutorService executorService =
options.as(ExecutorOptions.class).getScheduledExecutorService();
ExecutionStateSampler executionStateSampler =
- new ExecutionStateSampler(options, System::currentTimeMillis);
+ new ExecutionStateSampler(
+ options,
+ System::currentTimeMillis,
+ message -> {
+ System.err.println(
+ "\n*** FATAL ERROR: Timeout occurred! Exiting JVM with
status 1. ***");
+ System.err.println("Details: " + message);
+ System.exit(1);
Review Comment:
> instead of System.exit() we could have a completablefuture that we notify
and add this to what is waited on below on line 423
>
> something like:
>
> ```
> if (options.as(SdkHarnessOptions.class).getEnableLogViaFnApi()) {
> CompletableFuture.anyOf(control.terminationFuture(),
logging.terminationFuture(), samplerTerminationFuture).get();
> } else {
> CompletableFuture.anyOf(control.terminationFuture(),
samplerTerminationFuture).get();
> }
> ```
>
> That will let us take the normal shutdown path which has the benefit of
flushing the logging handler and other clean shutdown logic
I have several concerns.
1. If a CompleteableFuture field 'onFinish' is added to
ExecutionStateSampler, is the Consumer function still needed?
```ExecutionStateSampler executionStateSampler =
new ExecutionStateSampler(
options,
System::currentTimeMillis,
message -> {
String errMsg = "*** FATAL ERROR: Timeout occurred! Exiting
JVM with status 1. Details:" + message;
System.err.println(errMsg);
LOG.error(errMsg);
System.exit(1);
});```
2. Does the normal shutdown path allow to restart the pipeline job?
--
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-unsubscr...@beam.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
