This is an automated email from the ASF dual-hosted git repository. chia7712 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/yunikorn-core.git
The following commit(s) were added to refs/heads/master by this push: new 3bb3e711 [YUNIKORN-2445] Add comments around locking setup in tracker code (#816) 3bb3e711 is described below commit 3bb3e71148064420e62da5747ba29a76d31c03c8 Author: Yu-Lin Chen <kh87...@gmail.com> AuthorDate: Tue Feb 27 21:03:32 2024 +0800 [YUNIKORN-2445] Add comments around locking setup in tracker code (#816) Closes: #816 Signed-off-by: Chia-Ping Tsai <chia7...@gmail.com> --- pkg/scheduler/ugm/group_tracker.go | 2 ++ pkg/scheduler/ugm/queue_tracker.go | 15 +++++++++++++++ pkg/scheduler/ugm/user_tracker.go | 2 ++ 3 files changed, 19 insertions(+) diff --git a/pkg/scheduler/ugm/group_tracker.go b/pkg/scheduler/ugm/group_tracker.go index fc94febd..c03453fb 100644 --- a/pkg/scheduler/ugm/group_tracker.go +++ b/pkg/scheduler/ugm/group_tracker.go @@ -92,6 +92,7 @@ func (gt *GroupTracker) clearLimits(queuePath string) { gt.queueTracker.setLimit(strings.Split(queuePath, configs.DOT), nil, 0, false, group, false) } +// Note: headroom of queue tracker is not read-only, it also traverses the queue hierarchy and creates childQueueTracker if it does not exist. func (gt *GroupTracker) headroom(hierarchy []string) *resources.Resource { gt.Lock() defer gt.Unlock() @@ -159,6 +160,7 @@ func (gt *GroupTracker) decreaseAllTrackedResourceUsage(hierarchy []string) map[ return removedApplications } +// Note: canRunApp of queue tracker is not read-only, it also traverses the queue hierarchy and creates a childQueueTracker if it does not exist. func (gt *GroupTracker) canRunApp(hierarchy []string, applicationID string) bool { gt.Lock() defer gt.Unlock() diff --git a/pkg/scheduler/ugm/queue_tracker.go b/pkg/scheduler/ugm/queue_tracker.go index 830610fa..fe2177a7 100644 --- a/pkg/scheduler/ugm/queue_tracker.go +++ b/pkg/scheduler/ugm/queue_tracker.go @@ -28,6 +28,8 @@ import ( "github.com/apache/yunikorn-core/pkg/webservice/dao" ) +// The QueueTracker is designed to be lock free and should remain as such. +// Each QueueTracker object is always only linked to single UserTracker or GroupTracker. The responsibility of managing locks is delegated to those objects. type QueueTracker struct { queueName string queuePath string @@ -85,6 +87,7 @@ const ( group ) +// Note: Lock free call. The Lock of the linked tracker (UserTracker and GroupTracker) should be held before calling this function. func (qt *QueueTracker) increaseTrackedResource(hierarchy []string, applicationID string, trackType trackingType, usage *resources.Resource) bool { log.Log(log.SchedUGM).Debug("Increasing resource usage", zap.Int("tracking type", int(trackType)), @@ -122,6 +125,7 @@ func (qt *QueueTracker) increaseTrackedResource(hierarchy []string, applicationI return true } +// Note: Lock free call. The Lock of the linked tracker (UserTracker and GroupTracker) should be held before calling this function. func (qt *QueueTracker) decreaseTrackedResource(hierarchy []string, applicationID string, usage *resources.Resource, removeApp bool) (bool, bool) { log.Log(log.SchedUGM).Debug("Decreasing resource usage", zap.String("queue path", qt.queuePath), @@ -174,6 +178,7 @@ func (qt *QueueTracker) decreaseTrackedResource(hierarchy []string, applicationI return removeQT, true } +// Note: Lock free call. The Lock of the linked tracker (UserTracker and GroupTracker) should be held before calling this function. func (qt *QueueTracker) setLimit(hierarchy []string, maxResource *resources.Resource, maxApps uint64, useWildCard bool, trackType trackingType, doWildCardCheck bool) { log.Log(log.SchedUGM).Debug("Setting limits", zap.String("queue path", qt.queuePath), @@ -200,6 +205,8 @@ func (qt *QueueTracker) setLimit(hierarchy []string, maxResource *resources.Reso } } +// Note: Lock free call. The Lock of the linked tracker (UserTracker and GroupTracker) should be held before calling this function. +// Note: headroom is not read-only, it also traverses the queue hierarchy and creates childQueueTracker if it does not exist. func (qt *QueueTracker) headroom(hierarchy []string, trackType trackingType) *resources.Resource { // depth first: all the way to the leaf, create if not exists // more than 1 in the slice means we need to recurse down @@ -224,6 +231,7 @@ func (qt *QueueTracker) headroom(hierarchy []string, trackType trackingType) *re return resources.ComponentWiseMinPermissive(headroom, childHeadroom) } +// Note: Lock free call. The RLock of the linked tracker (UserTracker and GroupTracker) should be held before calling this function. func (qt *QueueTracker) getResourceUsageDAOInfo(parentQueuePath string) *dao.ResourceUsageDAOInfo { if qt == nil { return &dao.ResourceUsageDAOInfo{} @@ -250,6 +258,7 @@ func (qt *QueueTracker) getResourceUsageDAOInfo(parentQueuePath string) *dao.Res // IsQueuePathTrackedCompletely Traverse queue path upto the end queue through its linkage // to confirm entire queuePath has been tracked completely or not +// Note: Lock free call. The RLock of the linked tracker (UserTracker and GroupTracker) should be held before calling this function. func (qt *QueueTracker) IsQueuePathTrackedCompletely(hierarchy []string) bool { // depth first: all the way to the leaf, ignore if not exists // more than 1 in the slice means we need to recurse down @@ -271,6 +280,7 @@ func (qt *QueueTracker) IsQueuePathTrackedCompletely(hierarchy []string) bool { // linkage needs to be removed or not based on the running applications. // If there are any running applications in end leaf queue, we should remove the linkage between // the leaf and its parent queue using UnlinkQT method. Otherwise, we should leave as it is. +// Note: Lock free call. The RLock of the linked tracker (UserTracker and GroupTracker) should be held before calling this function. func (qt *QueueTracker) IsUnlinkRequired(hierarchy []string) bool { // depth first: all the way to the leaf, ignore if not exists // more than 1 in the slice means we need to recurse down @@ -295,6 +305,7 @@ func (qt *QueueTracker) IsUnlinkRequired(hierarchy []string) bool { // UnlinkQT Traverse queue path upto the end queue. If end queue has any more child queue trackers, // then goes upto each child queue and removes the linkage with its immediate parent +// Note: Lock free call. The Lock of the linked tracker (UserTracker and GroupTracker) should be held before calling this function. func (qt *QueueTracker) UnlinkQT(hierarchy []string) bool { log.Log(log.SchedUGM).Debug("Unlinking current queue tracker from its parent", zap.String("current queue ", qt.queueName), @@ -332,6 +343,7 @@ func (qt *QueueTracker) UnlinkQT(hierarchy []string) bool { // decreaseTrackedResourceUsageDownwards queuePath either could be parent or leaf queue path. // If it is parent queue path, then reset resourceUsage and runningApplications for all child queues, // If it is leaf queue path, reset resourceUsage and runningApplications for queue trackers in this queue path. +// Note: Lock free call. The Lock of the linked tracker (UserTracker and GroupTracker) should be held before calling this function. func (qt *QueueTracker) decreaseTrackedResourceUsageDownwards(hierarchy []string) map[string]bool { // depth first: all the way to the leaf, ignore if not exists // more than 1 in the slice means we need to recurse down @@ -359,6 +371,8 @@ func (qt *QueueTracker) decreaseTrackedResourceUsageDownwards(hierarchy []string return removedApplications } +// Note: Lock free call. The Lock of the linked tracker (UserTracker and GroupTracker) should be held before calling this function. +// Note: canRunApp is not read-only, it also traverses the queue hierarchy and creates a childQueueTracker if it does not exist. func (qt *QueueTracker) canRunApp(hierarchy []string, applicationID string, trackType trackingType) bool { // depth first: all the way to the leaf, create if not exists // more than 1 in the slice means we need to recurse down @@ -393,6 +407,7 @@ func (qt *QueueTracker) canRunApp(hierarchy []string, applicationID string, trac // canBeRemoved Start from root and reach all levels of queue hierarchy to confirm whether corresponding queue tracker // object can be removed from ugm or not. Based on running applications, resource usage, child queue trackers, max running apps, max resources etc // it decides the removal. It returns false the moment it sees any unexpected values for any queue in any levels. +// Note: Lock free call. The RLock of the linked tracker (UserTracker and GroupTracker) should be held before calling this function. func (qt *QueueTracker) canBeRemoved() bool { for _, childQT := range qt.childQueueTrackers { // quick check to avoid further traversal diff --git a/pkg/scheduler/ugm/user_tracker.go b/pkg/scheduler/ugm/user_tracker.go index 1f1ecfee..4ce5c3c5 100644 --- a/pkg/scheduler/ugm/user_tracker.go +++ b/pkg/scheduler/ugm/user_tracker.go @@ -144,6 +144,7 @@ func (ut *UserTracker) clearLimits(queuePath string, doWildCardCheck bool) { ut.queueTracker.setLimit(strings.Split(queuePath, configs.DOT), nil, 0, false, user, doWildCardCheck) } +// Note: headroom of queue tracker is not read-only, it also traverses the queue hierarchy and creates childQueueTracker if it does not exist. func (ut *UserTracker) headroom(hierarchy []string) *resources.Resource { ut.Lock() defer ut.Unlock() @@ -190,6 +191,7 @@ func (ut *UserTracker) canBeRemoved() bool { return ut.queueTracker.canBeRemoved() } +// Note: canRunApp of queue tracker is not read-only, it also traverses the queue hierarchy and creates a childQueueTracker if it does not exist. func (ut *UserTracker) canRunApp(hierarchy []string, applicationID string) bool { ut.Lock() defer ut.Unlock() --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@yunikorn.apache.org For additional commands, e-mail: issues-h...@yunikorn.apache.org