xintongsong commented on a change in pull request #11353: [FLINK-16438][yarn] 
Make YarnResourceManager starts workers using WorkerResourceSpec requested by 
SlotManager
URL: https://github.com/apache/flink/pull/11353#discussion_r403850488
 
 

 ##########
 File path: 
flink-yarn/src/main/java/org/apache/flink/yarn/YarnResourceManager.java
 ##########
 @@ -412,30 +439,32 @@ private void 
releaseFailedContainerAndRequestNewContainerIfRequired(ContainerId
 
                final ResourceID resourceId = new 
ResourceID(containerId.toString());
                // release the failed container
-               workerNodeMap.remove(resourceId);
+               YarnWorkerNode yarnWorkerNode = 
workerNodeMap.remove(resourceId);
                resourceManagerClient.releaseAssignedContainer(containerId);
                // and ask for a new one
-               requestYarnContainerIfRequired();
+               
requestYarnContainerIfRequired(yarnWorkerNode.getContainer().getResource());
        }
 
        private void returnExcessContainer(Container excessContainer) {
                log.info("Returning excess container {}.", 
excessContainer.getId());
                
resourceManagerClient.releaseAssignedContainer(excessContainer.getId());
        }
 
-       private void removeContainerRequest(AMRMClient.ContainerRequest 
pendingContainerRequest) {
-               numPendingContainerRequests--;
-
-               log.info("Removing container request {}. Pending container 
requests {}.", pendingContainerRequest, numPendingContainerRequests);
-
+       private void removeContainerRequest(AMRMClient.ContainerRequest 
pendingContainerRequest, WorkerResourceSpec workerResourceSpec) {
+               log.info("Removing container request {}.", 
pendingContainerRequest);
+               pendingWorkerCounter.decreaseAndGet(workerResourceSpec);
 
 Review comment:
   Unfortunately, we don't clean `workerSpecContainerResourceAdapter` up.
   
   I think the upper bond of the amount of records really depends on how many 
different `WorkerResourceSpec` do we have. If we want to clean the unused 
records up, the `WorkerSpecContainerResourceAdapter` will need 
`YarnResourceManager` to tell it which WorkerResourceSpec is no longer needed 
(all corresponding TMs are completed and no pending ones). I'm not sure whether 
this complexity is necessary.

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to