xintongsong commented on issue #11353: [FLINK-16438][yarn] Make 
YarnResourceManager starts workers using WorkerResourceSpec requested by 
SlotManager
URL: https://github.com/apache/flink/pull/11353#issuecomment-609595678
 
 
   @tillrohrmann, thanks for the review.
   
   Regarding hiding `pendingWorkerCounter` behind `ActiveResourceManager` 
methods, I think this is indeed a good suggestion. Just trying to confirm which 
methods the base class should expose before working on the implementation. I 
think the following methods would be good enough. WDYT?
   - `getNumPendingWorkers()`
   - `getNumPendingWorkers(WorkerResourceSpec workerResourceSpec)`
   - `notifyNewWorkerRequested(WorkerResourceSpec workerResourceSpec)`
   - `notifyNewWorkerAllocated(WorkerResourceSpec workerResourceSpec)`
   
   BTW, I was trying to split the changes for K8s/Yarn into 2 PRs (#11323 for 
K8s and this one for Yarn) to make the review easier. I mentioned in the PR 
description that this PR is based on #11323, but I guess it's not obvious 
enough. So, would you prefer that I address the comments related to k8s in 
#11323, or just keep them together in this PR? I'm ok with both ways.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to