pbacsko commented on a change in pull request #353:
URL:
https://github.com/apache/incubator-yunikorn-core/pull/353#discussion_r780134085
##########
File path: pkg/scheduler/partition.go
##########
@@ -380,7 +406,33 @@ func (pc *PartitionContext) removeApplication(appID
string) []*objects.Allocatio
// Remove app from queue
if queue := app.GetQueue(); queue != nil {
queue.RemoveApplication(app)
+
+ var updateGroupMetrics bool
+ if len(app.GetUser().User) > 0 {
+ user :=
queue.GetUserGroupManager().GetUser(app.GetUser().User)
+ if user != nil {
+ user.DecRunningApplications()
+ g :=
queue.GetUserGroupManager().GetGroup(user.GetUsedGroup())
+ if g != nil {
+ g.DecRunningApplications()
+ updateGroupMetrics = true
+ }
+ }
+ }
+
+ // Used when limit has been configured only for group, not for
individual user
+ if ! updateGroupMetrics && len(app.GetUser().User) > 0 {
+ for _, group := range app.GetUser().Groups {
+ // Is there any group (to which user belongs
to) config has limit settings?
+ g := queue.GetUserGroupManager().GetGroup(group)
+ if g != nil {
+ g.DecRunningApplications()
+ break
Review comment:
Ok, this might be a nitpick, but I looked at the existing group cache
code.
I think we might have an issue here. Let's say a user "john" who belongs to
"qe" group submitted an application which runs for a while. We have limits set
for this group. Then "john" is removed from "qe" and the group cache gets
updated. The application finishes but `DecRunningApplications()` will not be
called.
We need to store this information somewhere else, probably adding an extra
field to `Application` and not relying on `app.GetUser().Groups`.
--
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]
