XComp commented on code in PR #22341:
URL: https://github.com/apache/flink/pull/22341#discussion_r1295631119
##########
flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobMasterServiceLeadershipRunner.java:
##########
@@ -261,34 +261,69 @@ public void grantLeadership(UUID leaderSessionID) {
@GuardedBy("lock")
private void startJobMasterServiceProcessAsync(UUID leaderSessionId) {
- sequentialOperation =
- sequentialOperation.thenRun(
- () ->
- runIfValidLeader(
- leaderSessionId,
- ThrowingRunnable.unchecked(
- () ->
-
verifyJobSchedulingStatusAndCreateJobMasterServiceProcess(
-
leaderSessionId)),
- "verify job scheduling status and
create JobMasterServiceProcess"));
-
+ boolean isValid;
+ synchronized (lock) {
+ isValid = isValidLeader(leaderSessionId);
+ }
+ if (isValid) {
+ sequentialOperation =
+ sequentialOperation.thenCompose(
+ unused ->
+ runIfValidLeader(
+ leaderSessionId,
+ () ->
+
jobResultStore.hasJobResultEntryAsync(
+
getJobID()),
+ "verify jbb result entry")
+ .handle(
+ (hasJobResult, throwable)
-> {
+ if (hasJobResult ==
null) {
Review Comment:
The `if (hasJobResult == null) {` block becomes obsolete here (because there
shouldn't be a situation where no result is passed if no error happened in
upstream async calls for `handle(BiFunction)`. You could add a
`Precondition.checkNotNull` if you like. But the subsequent if call would cause
a `NullPointerException` anyway which makes the precondition obsolete again.
(... and a side-remark here: we would at least add logs rather than only
returning `null` in case if the `if (hasJobResult == null) {` case.)
--
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]
