(yunikorn-core) branch master updated: [YUNIKORN-3380] Fix user/group tracker quota leak on RemoveAllAllocations with pending asks (#1147)

[ma...@apache.org]

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 <he...@gmail.com>
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 <ma...@gmail.com>
---
 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: issues-unsubscribe@yunikorn.apache.org
For additional commands, e-mail: issues-help@yunikorn.apache.org