[GitHub] [flink] xintongsong commented on a change in pull request #12620: [FLINK-18226][runtime] Fix ActiveResourceManager requesting extra workers on termination of existing workers.

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

xintongsong commented on a change in pull request #12620:
URL: https://github.com/apache/flink/pull/12620#discussion_r439666292



##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManagerGateway.java
##########
@@ -104,13 +104,15 @@
 
 	/**
 	 * Sends the given {@link SlotReport} to the ResourceManager.
+	 * This method is only used for sending the initial slot report after a TaskManager registered to the ResourceManager.
+	 * After that, the heartbeat payload is used for periodically updating the slot reports.
 	 *
 	 * @param taskManagerRegistrationId id identifying the sending TaskManager
 	 * @param slotReport which is sent to the ResourceManager
 	 * @param timeout for the operation
 	 * @return Future which is completed with {@link Acknowledge} once the slot report has been received.
 	 */
-	CompletableFuture<Acknowledge> sendSlotReport(
+	CompletableFuture<Acknowledge> sendInitialSlotReport(

Review comment:
       I think this is not only about how it is used by the `TaskExecutor`, but also what this method does. The implementation of this method in `ResourceManager` actually registers the `TaskExecutor` at the `SlotManager`.
   
   However, this renaming is not absolutely necessary. I'm also ok with reverting it if consensus is not reached on this, given what we really want to fix this problem before the next RC.




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

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