[FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

[reswqa]

What is the purpose of the change

Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO.

Brief change log

• Only call isInGloballyTerminalState once in Dispatcher#submitJob.

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

• Dependencies (does it add or upgrade a dependency): no
• The public API, i.e., is any changed class annotated with @Public(Evolving): no
• The serializers: no
• The runtime per-record code paths (performance sensitive): no
• Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
• The S3 file system connector: no

Documentation

• Does this pull request introduce a new feature? no

[flinkbot]

CI report:

• 6fc03aa Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

• @flinkbot run azure re-run the last Azure build

[XComp]

Sorry for not getting back on that one earlier, @reswqa Thanks for working on it. I have a few minor comments. PTAL

[reswqa]

Sorry for not getting back on that one earlier.

It doesn't matter. Thanks @XComp for picking this up!

I'm confused by what you mean by "if it's safe enough". 🤔

The results returned may be different between multiple calls to isInGlobalyTerminalState (due to an entry may happened to be written to JobResultStore in the ioExecutor). In the implementation before the change, we can sense the change of this state faster (because multiple calls were made), and after the change, there is only one call. I don't think there should be any problems here, but I'm not very sure.

[reswqa]

I have updated this but seems that this pull request page does not perceive this new commit. This is strange because the corresponding fork branch(https://github.com/reswqa/flink/commits/FLINK-32098-reduce-io) has already been updated. I don't know if it's my network issue or Github's service issue.

[XComp]

I suspect it to be a Github issue. I'm gonna check again tomorrow

[reswqa]

@flinkbot run azure

[reswqa]

@flinkbot run azure

[reswqa]

It seems that CI is very unstable caused by https://issues.apache.org/jira/projects/FLINK/issues/FLINK-32632.

[XComp]

I'm looking into FLINK-32632 and will get back to you on that PR afterwards

[XComp]

LGTM 👍 Let's wait for CI to become stable again to have a final green CI run before merging.

[XComp]

@reswqa can you rebase the branch to most-recent master? FLINK-32632 is resolved.

[reswqa]

Thanks for your reminder and efforts to fix FLINK-32632 👍 I have rebased this to latest master.

[XComp]

The 1.18 release managers allowed to merge this change even after the feature freeze (see ML discussion). I'm going to go ahead and merge this change.