This is an automated email from the ASF dual-hosted git repository.
manirajv06 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 7b3650b4 [YUNIKORN-3380] Fix user/group tracker quota leak on
RemoveAllAllocations with pending asks (#1147)
7b3650b4 is described below
commit 7b3650b40c40e5988db2b76cea2cfbc8ed609e29
Author: hedger9487 <[email protected]>
AuthorDate: Fri Sep 4 11:49:45 2026 +0530
[YUNIKORN-3380] Fix user/group tracker quota leak on RemoveAllAllocations
with pending asks (#1147)
Closes: #1147
Signed-off-by: mani <[email protected]>
---
pkg/scheduler/objects/application.go | 4 ++--
pkg/scheduler/objects/application_test.go | 40 +++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/pkg/scheduler/objects/application.go
b/pkg/scheduler/objects/application.go
index 6d7bd6fd..09df0073 100644
--- a/pkg/scheduler/objects/application.go
+++ b/pkg/scheduler/objects/application.go
@@ -2237,8 +2237,8 @@ func (sa *Application) RemoveAllAllocations()
[]*Allocation {
// if an app doesn't have any allocations and the user doesn't have
other applications,
// the user tracker is nonexistent. We don't want to decrease resource
usage in this case.
- if ugm.GetUserManager().GetUserTracker(sa.user.User) != nil &&
resources.IsZero(sa.pending) {
- sa.decUserResourceUsage(resources.Add(sa.allocatedResource,
sa.allocatedPlaceholder), true)
+ if ugm.GetUserManager().GetUserTracker(sa.user.User) != nil {
+ sa.decUserResourceUsage(resources.Add(sa.allocatedResource,
sa.allocatedPlaceholder), resources.IsZero(sa.pending))
}
// cleanup allocated resource for app (placeholders and normal)
sa.allocatedResource = resources.NewResource()
diff --git a/pkg/scheduler/objects/application_test.go
b/pkg/scheduler/objects/application_test.go
index 6a28ddc5..1f42a3c5 100644
--- a/pkg/scheduler/objects/application_test.go
+++ b/pkg/scheduler/objects/application_test.go
@@ -955,6 +955,46 @@ func TestAllocations(t *testing.T) {
assertUserGroupResource(t, getTestUserGroup(), nil)
}
+func TestRemoveAllAllocationsWithPendingAsks(t *testing.T) {
+ setupUGM()
+ app := newApplication(appID1, "default", "root.a")
+ queue, err := createRootQueue(nil)
+ assert.NilError(t, err, "queue create failed")
+ app.queue = queue
+
+ resMap := map[string]string{"memory": "100", "vcores": "10"}
+ res, err := resources.NewResourceFromConf(resMap)
+ assert.NilError(t, err, "failed to create resource with error")
+
+ // 1. Add an allocation: UGM tracks 100m/10v
+ alloc := newAllocation(appID1, nodeID1, res)
+ app.AddAllocation(alloc)
+ assertUserGroupResource(t, getTestUserGroup(), res)
+
+ // 2. Add a pending ask so that app.pending is non-zero
+ ask := newAllocationAsk("ask-1", appID1, res)
+ err = app.AddAllocationAsk(ask)
+ assert.NilError(t, err)
+ assert.Assert(t, !resources.IsZero(app.GetPendingResource()), "app
should have pending resources")
+
+ // 3. Remove all allocations while pending ask is still present
+ allocs := app.RemoveAllAllocations()
+ assert.Equal(t, len(allocs), 1)
+ assert.Assert(t, resources.IsZero(app.GetAllocatedResource()), "app
allocated resources should be zero")
+
+ // 4. Invariant check: UGM usage must have decremented the released
allocation!
+ // In the unpatched code, this assertion will fail because
decUserResourceUsage was skipped!
+ assertUserGroupResource(t, getTestUserGroup(), nil)
+
+ // 5. App should still have its pending ask and remain in Accepted
state (not Completing)
+ assert.Assert(t, resources.Equals(app.GetPendingResource(), res),
"pending resources should remain intact")
+ assert.Equal(t, app.CurrentState(), Accepted.String(), "app should
remain in Accepted state while asks are pending")
+
+ // 6. Now remove the pending ask and verify pending resources are
cleared
+ app.RemoveAllocationAsk(ask.GetAllocationKey())
+ assert.Assert(t, resources.IsZero(app.GetPendingResource()), "pending
resources should be zero after ask removal")
+}
+
func TestGangAllocChange(t *testing.T) {
resMap := map[string]string{"first": "4"}
totalPH, err := resources.NewResourceFromConf(resMap)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
