[GitHub] [incubator-yunikorn-core] wilfred-s commented on pull request #377: [YUNIKORN-1099] Fix deadlock in Application.notifyRMAllocationReleased()

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

wilfred-s commented on pull request #377:
URL: https://github.com/apache/incubator-yunikorn-core/pull/377#issuecomment-1056562159


   >I am not sure whether I communicated clearly. Just to make it explicit, I meant, application#notifyRMAllocationReleased() and context#notifyRMAllocationReleased() methods.
   
   The two duplicate functions cannot be avoided. The application is in a sub package from the scheduler, objects. The scheduler imports that package. That means the application cannot contain the context. To do that would mean importing the scheduler package. That is a circular dependency.
   That dependency is worked around by keeping a reference to the RMEventHandler in the application.
   
   > Also, Should we need to clear cache in case of RMReleaseAllocationAskEvent events (application#notifyRMAllocationAskReleased() )?
   
   An ask is a pod that has not been scheduled yet. So it is a "new" pod that was created. That pod might or might not be in the cache just yet. It surely is not linked to a node. The only way that a pod is normally removed in that state is if it comes from the shim to the core. The only exception is a placeholder timeout. A placeholder that has not been placed on a node.
   We should not be leaving anything around at the moment in the cache. Even for placeholders that timeout we clean up including the caches through the placeholder manager.


-- 
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: reviews-unsubscribe@yunikorn.apache.org

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