lixmgl commented on code in PR #429:
URL: https://github.com/apache/yunikorn-core/pull/429#discussion_r954325378


##########
pkg/scheduler/objects/queue.go:
##########
@@ -1292,3 +1304,67 @@ func (sq *Queue) String() string {
        return fmt.Sprintf("{QueuePath: %s, State: %s, StateTime: %x, 
MaxResource: %s}",
                sq.QueuePath, sq.stateMachine.Current(), sq.stateTime, 
sq.maxResource)
 }
+
+func (sq *Queue) incRunningApps() {
+       if sq == nil {
+               return
+       }
+       if sq.parent != nil {
+               sq.parent.incRunningApps()
+       }
+       sq.internalIncRunningApps()
+}
+
+func (sq *Queue) internalIncRunningApps() {
+       sq.Lock()
+       defer sq.Unlock()
+       sq.runningApps++
+}

Review Comment:
   The reason I separated this logic into a new method instead of merging into 
incRunningApps() is: 
   Based on my understanding, instruction order is not ensured in golang even 
with lock  (refer: 
https://stackoverflow.com/questions/60340382/can-i-use-lock-to-ensure-instruction-order)
   If we merge into incRunningApps(), sq.Lock() could get executed before this 
recursion. Each parent could be locked during recursion.  
   if sq.parent != nil {
                ok = sq.parent.canRun()
   }



##########
pkg/scheduler/objects/queue.go:
##########
@@ -1292,3 +1304,67 @@ func (sq *Queue) String() string {
        return fmt.Sprintf("{QueuePath: %s, State: %s, StateTime: %x, 
MaxResource: %s}",
                sq.QueuePath, sq.stateMachine.Current(), sq.stateTime, 
sq.maxResource)
 }
+
+func (sq *Queue) incRunningApps() {
+       if sq == nil {
+               return
+       }
+       if sq.parent != nil {
+               sq.parent.incRunningApps()
+       }
+       sq.internalIncRunningApps()
+}
+
+func (sq *Queue) internalIncRunningApps() {
+       sq.Lock()
+       defer sq.Unlock()
+       sq.runningApps++
+}
+
+func (sq *Queue) decRunningApps() {
+       if sq == nil {
+               return
+       }
+       if sq.parent != nil {
+               sq.parent.decRunningApps()
+       }
+       sq.internalDecRunningApps()
+}
+
+func (sq *Queue) internalDecRunningApps() {
+       sq.Lock()
+       defer sq.Unlock()
+       sq.runningApps--
+}

Review Comment:
   same reason as before.



-- 
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]

Reply via email to