[GitHub] [flink] KarmaGYZ commented on a diff in pull request #19840: [FLINK-24713][Runtime/Coordination] Support the initial delay for SlotManager to wait fo…

[GitBox <gi...@apache.org>]

KarmaGYZ commented on code in PR #19840:
URL: https://github.com/apache/flink/pull/19840#discussion_r917500734


##########
flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/slotmanager/FineGrainedSlotManager.java:
##########
@@ -504,8 +522,11 @@ private void checkResourceRequirementsWithDelay() {
                                             Preconditions.checkNotNull(requirementsCheckFuture)
                                                     .complete(null);
                                         }),
-                        requirementsCheckDelay.toMillis(),
+                        delay,
                         TimeUnit.MILLISECONDS);
+                isRequirementsCheckLongDelay = false;
+            } else {
+                checkResourceRequirements();

Review Comment:
   > I have tried this way, but I found it will change this method's behavior to async when delay is 0, now the behavior is sync. So I keep the current implementation and most test do not have to be touched.
   
   I don't think this method by design is guaranteed to be triggered synchronously when the delay is zero.
   
   It's always good to do deduplication work. But I think the `DeclarativeSlotManager` will be entirely replaced by the `FineGrainedSlotManager` in the near future. So, we do not need to do it atm.



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