zhuzhurk commented on code in PR #22506: URL: https://github.com/apache/flink/pull/22506#discussion_r1186656051
########## flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobMaster.java: ########## @@ -473,26 +500,50 @@ public CompletableFuture<Acknowledge> cancel(Time timeout) { @Override public CompletableFuture<Acknowledge> updateTaskExecutionState( final TaskExecutionState taskExecutionState) { - FlinkException taskExecutionException; + checkNotNull(taskExecutionState, "taskExecutionState"); + // Use the main/caller thread for all updates to make sure they are processed in order. + // (MainThreadExecutor i.e., the akka thread pool does not guarantee that) + // Only detach for a FAILED state update that is terminal and may perform io heavy labeling. + if (ExecutionState.FAILED.equals(taskExecutionState.getExecutionState())) { + return labelFailure(taskExecutionState) + .thenApplyAsync( + taskStateWithLabels -> { + try { + return doUpdateTaskExecutionState(taskStateWithLabels); + } catch (FlinkException e) { + throw new CompletionException(e); + } + }, + getMainThreadExecutor()); + } try { - checkNotNull(taskExecutionState, "taskExecutionState"); + return CompletableFuture.completedFuture( + doUpdateTaskExecutionState(taskExecutionState)); + } catch (FlinkException e) { + return FutureUtils.completedExceptionally(e); + } + } + private Acknowledge doUpdateTaskExecutionState(final TaskExecutionState taskExecutionState) + throws FlinkException { + @Nullable FlinkException taskExecutionException; + try { if (schedulerNG.updateTaskExecutionState(taskExecutionState)) { Review Comment: A new thought just came up to me. Since the failure labeling is not a critical process, it's better that the failure recovery process will not get blocked due to it. So maybe it's better to just record the `CompletableFuture<Map<String, String>>` in `ExceptionHistoryEntry`. When the exception history is requested, the future can be converted into a `Map<String, String>` if it is completed, otherwise it can be converted into an empty map which will be shown as `Label Unknown Yet` on the web UI. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org