manirajv06 commented on PR #1096: URL: https://github.com/apache/yunikorn-core/pull/1096#issuecomment-4992313334
> As far as I can see, you added `preAllocateConditions()` to `tryNodes()`, `tryNodesNoReserve()`, `tryPlaceholderAllocate()` and `Preemptor.tryNodes()`. But `tryRequiredNode()` and `tryReservedAllocate()` are missing. Ignored it as I thought of not doing predicate checks and have to make it at any cost. But then going through the flow again, felt that predicate checks are still required too and makes sense. So, made changes. @pbacsko @wilfred-s I am working on the other review comments. Will cover it up in next commit. Q: As of now, tryPreemption() for required node does not carry out preemption predicate checks unlike Intra queue preemption does it. I think we need to do even here. Thoughts? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
