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

Reply via email to