You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "reswqa (via GitHub)" <gi...@apache.org> on 2023/05/15 12:49:06 UTC

[GitHub] [flink] reswqa opened a new pull request, #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

reswqa opened a new pull request, #22585:
URL: https://github.com/apache/flink/pull/22585

   ## 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
   


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] reswqa commented on a diff in pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "reswqa (via GitHub)" <gi...@apache.org>.
reswqa commented on code in PR #22585:
URL: https://github.com/apache/flink/pull/22585#discussion_r1193794034


##########
flink-runtime/src/main/java/org/apache/flink/runtime/dispatcher/Dispatcher.java:
##########
@@ -507,28 +507,29 @@ public CompletableFuture<Acknowledge> submitJob(JobGraph jobGraph, Time timeout)
                 "Received JobGraph submission '{}' ({}).", jobGraph.getName(), jobGraph.getJobID());
 
         try {
-            if (isDuplicateJob(jobGraph.getJobID())) {
-                if (isInGloballyTerminalState(jobGraph.getJobID())) {
+            JobID jobID = jobGraph.getJobID();
+            boolean inGloballyTerminalState = isInGloballyTerminalState(jobGraph.getJobID());

Review Comment:
   Not sure if it's safe enough here. However, it seems that there was no guarantee before that the write operation of `JobResultStore` in this method would not occur simultaneously, but it does guarantee more strong visibility.



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] reswqa commented on a diff in pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "reswqa (via GitHub)" <gi...@apache.org>.
reswqa commented on code in PR #22585:
URL: https://github.com/apache/flink/pull/22585#discussion_r1193794034


##########
flink-runtime/src/main/java/org/apache/flink/runtime/dispatcher/Dispatcher.java:
##########
@@ -507,28 +507,29 @@ public CompletableFuture<Acknowledge> submitJob(JobGraph jobGraph, Time timeout)
                 "Received JobGraph submission '{}' ({}).", jobGraph.getName(), jobGraph.getJobID());
 
         try {
-            if (isDuplicateJob(jobGraph.getJobID())) {
-                if (isInGloballyTerminalState(jobGraph.getJobID())) {
+            JobID jobID = jobGraph.getJobID();
+            boolean inGloballyTerminalState = isInGloballyTerminalState(jobGraph.getJobID());

Review Comment:
   Not sure if it's safe enough here. However, it seems that there was no guarantee before that the write operation of `JobResultStore` in this method would not occur simultaneously, but it does guarantee more strong visibility. 🤔 



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] reswqa commented on a diff in pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "reswqa (via GitHub)" <gi...@apache.org>.
reswqa commented on code in PR #22585:
URL: https://github.com/apache/flink/pull/22585#discussion_r1269520503


##########
flink-runtime/src/main/java/org/apache/flink/runtime/dispatcher/Dispatcher.java:
##########
@@ -510,32 +510,34 @@ private void stopDispatcherServices() throws Exception {
 
     @Override
     public CompletableFuture<Acknowledge> submitJob(JobGraph jobGraph, Time timeout) {
-        log.info(
-                "Received JobGraph submission '{}' ({}).", jobGraph.getName(), jobGraph.getJobID());
+        final JobID jobID = jobGraph.getJobID();
+        log.info("Received JobGraph submission '{}' ({}).", jobGraph.getName(), jobID);
 
         try {
-            if (isDuplicateJob(jobGraph.getJobID())) {
-                if (isInGloballyTerminalState(jobGraph.getJobID())) {
+            final boolean inGloballyTerminalState = isInGloballyTerminalState(jobID);
+            if (inGloballyTerminalState
+                    || jobManagerRunnerRegistry.isRegistered(jobID)
+                    || submittedAndWaitingTerminationJobIDs.contains(jobID)) {
+                if (inGloballyTerminalState) {
                     log.warn(
                             "Ignoring JobGraph submission '{}' ({}) because the job already reached a globally-terminal state (i.e. {}) in a previous execution.",
                             jobGraph.getName(),
-                            jobGraph.getJobID(),
+                            jobID,
                             Arrays.stream(JobStatus.values())
                                     .filter(JobStatus::isGloballyTerminalState)
                                     .map(JobStatus::name)
                                     .collect(Collectors.joining(", ")));
                 }
 
                 final DuplicateJobSubmissionException exception =
-                        isInGloballyTerminalState(jobGraph.getJobID())
-                                ? DuplicateJobSubmissionException.ofGloballyTerminated(
-                                        jobGraph.getJobID())
-                                : DuplicateJobSubmissionException.of(jobGraph.getJobID());
+                        inGloballyTerminalState
+                                ? DuplicateJobSubmissionException.ofGloballyTerminated(jobID)
+                                : DuplicateJobSubmissionException.of(jobID);
                 return FutureUtils.completedExceptionally(exception);

Review Comment:
   Good point, this can greatly simplify the code. 👍 



##########
flink-runtime/src/main/java/org/apache/flink/runtime/dispatcher/Dispatcher.java:
##########
@@ -510,32 +510,34 @@ private void stopDispatcherServices() throws Exception {
 
     @Override
     public CompletableFuture<Acknowledge> submitJob(JobGraph jobGraph, Time timeout) {
-        log.info(
-                "Received JobGraph submission '{}' ({}).", jobGraph.getName(), jobGraph.getJobID());
+        final JobID jobID = jobGraph.getJobID();
+        log.info("Received JobGraph submission '{}' ({}).", jobGraph.getName(), jobID);
 
         try {
-            if (isDuplicateJob(jobGraph.getJobID())) {
-                if (isInGloballyTerminalState(jobGraph.getJobID())) {
+            final boolean inGloballyTerminalState = isInGloballyTerminalState(jobID);
+            if (inGloballyTerminalState
+                    || jobManagerRunnerRegistry.isRegistered(jobID)
+                    || submittedAndWaitingTerminationJobIDs.contains(jobID)) {
+                if (inGloballyTerminalState) {
                     log.warn(
                             "Ignoring JobGraph submission '{}' ({}) because the job already reached a globally-terminal state (i.e. {}) in a previous execution.",
                             jobGraph.getName(),
-                            jobGraph.getJobID(),
+                            jobID,
                             Arrays.stream(JobStatus.values())
                                     .filter(JobStatus::isGloballyTerminalState)
                                     .map(JobStatus::name)
                                     .collect(Collectors.joining(", ")));
                 }
 
                 final DuplicateJobSubmissionException exception =
-                        isInGloballyTerminalState(jobGraph.getJobID())
-                                ? DuplicateJobSubmissionException.ofGloballyTerminated(
-                                        jobGraph.getJobID())
-                                : DuplicateJobSubmissionException.of(jobGraph.getJobID());
+                        inGloballyTerminalState
+                                ? DuplicateJobSubmissionException.ofGloballyTerminated(jobID)
+                                : DuplicateJobSubmissionException.of(jobID);
                 return FutureUtils.completedExceptionally(exception);

Review Comment:
   Good point, this can greatly simplify the code. 👍 



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] XComp commented on a diff in pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "XComp (via GitHub)" <gi...@apache.org>.
XComp commented on code in PR #22585:
URL: https://github.com/apache/flink/pull/22585#discussion_r1263591712


##########
flink-runtime/src/main/java/org/apache/flink/runtime/dispatcher/Dispatcher.java:
##########
@@ -507,28 +507,29 @@ public CompletableFuture<Acknowledge> submitJob(JobGraph jobGraph, Time timeout)
                 "Received JobGraph submission '{}' ({}).", jobGraph.getName(), jobGraph.getJobID());
 
         try {
-            if (isDuplicateJob(jobGraph.getJobID())) {
-                if (isInGloballyTerminalState(jobGraph.getJobID())) {
+            JobID jobID = jobGraph.getJobID();
+            boolean inGloballyTerminalState = isInGloballyTerminalState(jobGraph.getJobID());

Review Comment:
   I'm confused by what you mean by "if it's safe enough". :thinking: 



##########
flink-runtime/src/main/java/org/apache/flink/runtime/dispatcher/Dispatcher.java:
##########
@@ -507,28 +507,29 @@ public CompletableFuture<Acknowledge> submitJob(JobGraph jobGraph, Time timeout)
                 "Received JobGraph submission '{}' ({}).", jobGraph.getName(), jobGraph.getJobID());
 
         try {
-            if (isDuplicateJob(jobGraph.getJobID())) {
-                if (isInGloballyTerminalState(jobGraph.getJobID())) {
+            JobID jobID = jobGraph.getJobID();
+            boolean inGloballyTerminalState = isInGloballyTerminalState(jobGraph.getJobID());

Review Comment:
   ```suggestion
               boolean inGloballyTerminalState = isInGloballyTerminalState(jobID);
   ```



##########
flink-runtime/src/main/java/org/apache/flink/runtime/dispatcher/Dispatcher.java:
##########
@@ -507,28 +507,29 @@ public CompletableFuture<Acknowledge> submitJob(JobGraph jobGraph, Time timeout)
                 "Received JobGraph submission '{}' ({}).", jobGraph.getName(), jobGraph.getJobID());
 
         try {
-            if (isDuplicateJob(jobGraph.getJobID())) {
-                if (isInGloballyTerminalState(jobGraph.getJobID())) {
+            JobID jobID = jobGraph.getJobID();
+            boolean inGloballyTerminalState = isInGloballyTerminalState(jobGraph.getJobID());

Review Comment:
   ```suggestion
               final JobID jobID = jobGraph.getJobID();
               final boolean inGloballyTerminalState = isInGloballyTerminalState(jobGraph.getJobID());
   ```
   nit



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] reswqa commented on pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "reswqa (via GitHub)" <gi...@apache.org>.
reswqa commented on PR #22585:
URL: https://github.com/apache/flink/pull/22585#issuecomment-1644997041

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


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] XComp commented on pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "XComp (via GitHub)" <gi...@apache.org>.
XComp commented on PR #22585:
URL: https://github.com/apache/flink/pull/22585#issuecomment-1659944337

   The 1.18 release managers allowed to merge this change even after the feature freeze (see [ML discussion](https://lists.apache.org/thread/b232d9or85vlorrbl41916rrdk6c4bwh)). I'm going to go ahead and merge this change.


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] XComp commented on pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "XComp (via GitHub)" <gi...@apache.org>.
XComp commented on PR #22585:
URL: https://github.com/apache/flink/pull/22585#issuecomment-1649778136

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


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] reswqa commented on pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "reswqa (via GitHub)" <gi...@apache.org>.
reswqa commented on PR #22585:
URL: https://github.com/apache/flink/pull/22585#issuecomment-1644014056

   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.


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] XComp merged pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "XComp (via GitHub)" <gi...@apache.org>.
XComp merged PR #22585:
URL: https://github.com/apache/flink/pull/22585


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] reswqa commented on pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "reswqa (via GitHub)" <gi...@apache.org>.
reswqa commented on PR #22585:
URL: https://github.com/apache/flink/pull/22585#issuecomment-1649828790

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


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] flinkbot commented on pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #22585:
URL: https://github.com/apache/flink/pull/22585#issuecomment-1547809722

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "8066c5b505d2e6e6120e868604f47c1fa4068719",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "8066c5b505d2e6e6120e868604f47c1fa4068719",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 8066c5b505d2e6e6120e868604f47c1fa4068719 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] reswqa commented on pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "reswqa (via GitHub)" <gi...@apache.org>.
reswqa commented on PR #22585:
URL: https://github.com/apache/flink/pull/22585#issuecomment-1644884152

   @flinkbot run azure


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] XComp commented on a diff in pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "XComp (via GitHub)" <gi...@apache.org>.
XComp commented on code in PR #22585:
URL: https://github.com/apache/flink/pull/22585#discussion_r1269389225


##########
flink-runtime/src/main/java/org/apache/flink/runtime/dispatcher/Dispatcher.java:
##########
@@ -510,32 +510,34 @@ private void stopDispatcherServices() throws Exception {
 
     @Override
     public CompletableFuture<Acknowledge> submitJob(JobGraph jobGraph, Time timeout) {
-        log.info(
-                "Received JobGraph submission '{}' ({}).", jobGraph.getName(), jobGraph.getJobID());
+        final JobID jobID = jobGraph.getJobID();
+        log.info("Received JobGraph submission '{}' ({}).", jobGraph.getName(), jobID);
 
         try {
-            if (isDuplicateJob(jobGraph.getJobID())) {
-                if (isInGloballyTerminalState(jobGraph.getJobID())) {
+            final boolean inGloballyTerminalState = isInGloballyTerminalState(jobID);
+            if (inGloballyTerminalState
+                    || jobManagerRunnerRegistry.isRegistered(jobID)
+                    || submittedAndWaitingTerminationJobIDs.contains(jobID)) {
+                if (inGloballyTerminalState) {
                     log.warn(
                             "Ignoring JobGraph submission '{}' ({}) because the job already reached a globally-terminal state (i.e. {}) in a previous execution.",
                             jobGraph.getName(),
-                            jobGraph.getJobID(),
+                            jobID,
                             Arrays.stream(JobStatus.values())
                                     .filter(JobStatus::isGloballyTerminalState)
                                     .map(JobStatus::name)
                                     .collect(Collectors.joining(", ")));
                 }
 
                 final DuplicateJobSubmissionException exception =
-                        isInGloballyTerminalState(jobGraph.getJobID())
-                                ? DuplicateJobSubmissionException.ofGloballyTerminated(
-                                        jobGraph.getJobID())
-                                : DuplicateJobSubmissionException.of(jobGraph.getJobID());
+                        inGloballyTerminalState
+                                ? DuplicateJobSubmissionException.ofGloballyTerminated(jobID)
+                                : DuplicateJobSubmissionException.of(jobID);
                 return FutureUtils.completedExceptionally(exception);

Review Comment:
   What about the following:
   ```
               if (isInGloballyTerminalState(jobID)) {
                   log.warn(
                           "Ignoring JobGraph submission '{}' ({}) because the job already reached a globally-terminal state (i.e. {}) in a previous execution.",
                           jobGraph.getName(),
                           jobID,
                           Arrays.stream(JobStatus.values())
                                   .filter(JobStatus::isGloballyTerminalState)
                                   .map(JobStatus::name)
                                   .collect(Collectors.joining(", ")));
   
                   return FutureUtils.completedExceptionally(
                           DuplicateJobSubmissionException.ofGloballyTerminated(jobID));
               } else if (jobManagerRunnerRegistry.isRegistered(jobID)
                       || submittedAndWaitingTerminationJobIDs.contains(jobID)) {
                   // job with the given jobID is not terminated, yet
                   return FutureUtils.completedExceptionally(
                           DuplicateJobSubmissionException.of(jobID));
   ```
   I just noticed that we could improve the if condition. That would require only a single method call. WDYT?



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] reswqa commented on pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "reswqa (via GitHub)" <gi...@apache.org>.
reswqa commented on PR #22585:
URL: https://github.com/apache/flink/pull/22585#issuecomment-1644956935

   @flinkbot run azure


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] XComp commented on pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "XComp (via GitHub)" <gi...@apache.org>.
XComp commented on PR #22585:
URL: https://github.com/apache/flink/pull/22585#issuecomment-1645544432

   I'm looking into [FLINK-32632](https://issues.apache.org/jira/browse/FLINK-32632) and will get back to you on that PR afterwards


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] reswqa commented on pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "reswqa (via GitHub)" <gi...@apache.org>.
reswqa commented on PR #22585:
URL: https://github.com/apache/flink/pull/22585#issuecomment-1637408260

   > 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.
   


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] XComp commented on pull request #22585: [FLINK-32098][runtime] Dispatcher#submitJob calls Dispatcher#isInGloballyTerminalState up to three times which might be expensive due to IO

Posted by "XComp (via GitHub)" <gi...@apache.org>.
XComp commented on PR #22585:
URL: https://github.com/apache/flink/pull/22585#issuecomment-1644075908

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


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org