chenyulin0719 commented on PR #841: URL: https://github.com/apache/yunikorn-core/pull/841#issuecomment-2051615749
Below 2 function won't inference each other because they're both holding queue lock before or after this PR. 1. (sq *Queue) RemoveQueue() -> RLock (Before this PR, it called sq.IsManaged() in sendRemoveQueueEvent(), which also hold RLock) 2. (sq *Queue) ApplyConf() -> WLock https://github.com/apache/yunikorn-core/blob/16d2c9cd79e73be51d23f0ce5c5d9f153be140bc/pkg/scheduler/objects/queue.go#L303-L307 After rethinking it, I realized that I was wrong. Some functions call queue.IsManaged() (e.g. sendRemoveQueueEvent) while holding the queue lock, but others do not. I suddenly have a crazy idea. How about creating lock-free version of IsManaged()? 1. IsManaged() 2. IsManagedNoLock() -> Lock free, for sendRemoveQueueEvent() The pros is that we can pass a *Queue to queueEvents again, which eliminates the need for repeated parameters. However, I haven't seen any similar implementations before, maybe it's not a good idea for dealing lock. (I know the benefit is quite small, it's a matter of preference. I'm good with this PR.) -- 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]
