manirajv06 commented on a change in pull request #353:
URL: 
https://github.com/apache/incubator-yunikorn-core/pull/353#discussion_r804876747



##########
File path: pkg/scheduler/partition.go
##########
@@ -338,6 +338,50 @@ func (pc *PartitionContext) AddApplication(app 
*objects.Application) error {
                return fmt.Errorf("failed to find queue %s for application %s", 
queueName, appID)
        }
 
+       if app.GetUser().User != "" {
+               // Has user limit configured with "*" ?
+               user := queue.GetUserGroupManager().GetUser(app.GetUser().User)
+               if queue.GetUserGroupManager().IsAllUserAllowed() && user == 
nil {
+                       user = objects.NewUser(app.GetUser().User)
+                       
user.SetMaxApplications(queue.GetUserGroupManager().GetUser(objects.AllUser).GetMaxApplications())
+                       queue.GetUserGroupManager().AddUserIfAbsent(user)
+               }
+
+               // Has user or group defined with any limits? If yes, Is there 
a room to operate?
+               user = queue.GetUserGroupManager().GetUser(app.GetUser().User)
+               if user != nil && user.IsMaxAppsLimitSet() {
+                       if user.IsRunningAppsUnderLimit() {
+                               user.IncRunningApplications()

Review comment:
       Thank you for your effort in putting this together. Really appreciate.
   
   I spent some time on trying "overrun/slips" cases by replacing condition 
used in Lineno. 65 with ! IsRunningAppsUnderLimit() in your first program as 
load happens in a safer & correct way and also whole execution happens with in 
"user" object context. Yes, slips are happening very rarely. For example, 8-15 
times in 10k runs. Also, tried with IsRunningAppsUnderLimit() (without 
negation) and finding results smoother as condition (the other way) in 
IsRunningAppsUnderLimit() is defensive and safeguarding against us getting into 
bad situation.
   
   On the other side, yes, I too had come across safer CAS before, which is 
safer relatively and comes up with a cost of running for loop multiple runs and 
seems like implementing a lock in a slightly different way.
   
   On top of all these points, there are uncertainties about this whole 
sync/atomic package observing from these threads:
   1. https://github.com/golang/go/issues/5045
   2. https://research.swtch.com/gomm
   
   Will update more on this later. Please share your thoughts as well.




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