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-k8shim.git
The following commit(s) were added to refs/heads/master by this push:
new 6b777b40 [YUNIKORN-3354] Recovery pod ordering is not reproducible
despite intending to be (#1075)
6b777b40 is described below
commit 6b777b40ae20be564352080c621e8fe376d64e05
Author: PoiBlackTea <[email protected]>
AuthorDate: Mon Aug 24 17:18:51 2026 +0530
[YUNIKORN-3354] Recovery pod ordering is not reproducible despite intending
to be (#1075)
During scheduler recovery, Context.registerPods() sorts recovered pods with
the intention of making queue ordering consistent across restarts. However,
because CreationTimestamp has 1-second resolution at source and the informer
list order is non-deterministic (Go map iteration), sorting on
CreationTimestamp alone produces arbitrary ordering when multiple pods share
the exact same second. The same non-deterministic ordering occurs during
per-application task dispatch in Application.ge [...]
This PR fixes the issue by using UID (taskID) as a deterministic secondary
tie-breaker when CreationTimestamp values are identical.
Closes: #1075
Signed-off-by: Manikandan R <[email protected]>
---
pkg/cache/application.go | 5 ++++-
pkg/cache/application_test.go | 10 +++++++---
pkg/cache/context.go | 5 ++++-
pkg/cache/context_test.go | 9 +++++++--
4 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/pkg/cache/application.go b/pkg/cache/application.go
index eaa4af89..c7fd3c36 100644
--- a/pkg/cache/application.go
+++ b/pkg/cache/application.go
@@ -310,7 +310,10 @@ func (app *Application) getTasks(state string) []*Task {
sort.Slice(taskList, func(i, j int) bool {
l := taskList[i]
r := taskList[j]
- return l.createTime.Before(r.createTime)
+ if l.createTime.Unix() != r.createTime.Unix() {
+ return l.createTime.Unix() < r.createTime.Unix()
+ }
+ return l.taskID < r.taskID
})
return taskList
diff --git a/pkg/cache/application_test.go b/pkg/cache/application_test.go
index 5aa0e078..85a8dfeb 100644
--- a/pkg/cache/application_test.go
+++ b/pkg/cache/application_test.go
@@ -848,14 +848,18 @@ func TestTryReservePostRestart(t *testing.T) {
},
})
+ // add tasks in arbitrary order (task2 before task1) to test
deterministic sorting by taskID
app.addTask(task0)
- app.addTask(task1)
app.addTask(task2)
+ app.addTask(task1)
// there should be 1 Allocated task, i.e task0
- // there should be 2 New tasks, i.e task1 and task2
+ // there should be 2 New tasks, i.e task1 and task2 sorted by taskID
assert.Equal(t, len(app.getTasks(TaskStates().Allocated)), 1)
- assert.Equal(t, len(app.getTasks(TaskStates().New)), 2)
+ newTasks := app.getTasks(TaskStates().New)
+ assert.Equal(t, len(newTasks), 2)
+ assert.Equal(t, newTasks[0].GetTaskID(), "task01", "tasks with
identical creation time should be sorted deterministically by taskID: expected
task01 at index 0")
+ assert.Equal(t, newTasks[1].GetTaskID(), "task02", "tasks with
identical creation time should be sorted deterministically by taskID: expected
task02 at index 1")
// run app schedule
app.Schedule()
diff --git a/pkg/cache/context.go b/pkg/cache/context.go
index 9b99be02..9ce9e6a5 100644
--- a/pkg/cache/context.go
+++ b/pkg/cache/context.go
@@ -1731,7 +1731,10 @@ func (ctx *Context) registerPods() ([]*v1.Pod, error) {
// sort pods by creation time so that overall queue ordering is
consistent with prior runs
sort.Slice(pods, func(i, j int) bool {
- return pods[i].CreationTimestamp.Unix() <
pods[j].CreationTimestamp.Unix()
+ if pods[i].CreationTimestamp.Unix() !=
pods[j].CreationTimestamp.Unix() {
+ return pods[i].CreationTimestamp.Unix() <
pods[j].CreationTimestamp.Unix()
+ }
+ return pods[i].UID < pods[j].UID
})
// add all pods to the context
diff --git a/pkg/cache/context_test.go b/pkg/cache/context_test.go
index 226dee53..384f1b49 100644
--- a/pkg/cache/context_test.go
+++ b/pkg/cache/context_test.go
@@ -2635,14 +2635,19 @@ func TestRegisterPods(t *testing.T) {
pod3 := newPodHelper(appID3, namespace, uid3, nodeName1, appID3,
v1.PodSucceeded)
pod4 := newPodHelper(appID4, namespace, uid4, nodeName1, appID4,
v1.PodRunning)
+ // add pods in arbitrary order to test deterministic sorting by UID
when creation timestamps are identical
+ api.GetPodListerMock().AddPod(pod4)
api.GetPodListerMock().AddPod(pod1)
- api.GetPodListerMock().AddPod(pod2)
api.GetPodListerMock().AddPod(pod3)
- api.GetPodListerMock().AddPod(pod4)
+ api.GetPodListerMock().AddPod(pod2)
pods, err = context.registerPods()
assert.NilError(t, err, "register pods should not have failed")
assert.Assert(t, assertListerPods(pods, 3), "should have returned 3
running pods in the list")
+ assert.Equal(t, string(pods[0].UID), uid1, "pods with identical
creation time should be sorted deterministically by UID: expected pod1 at index
0")
+ assert.Equal(t, string(pods[1].UID), uid2, "pods with identical
creation time should be sorted deterministically by UID: expected pod2 at index
1")
+ assert.Assert(t, pods[2] == nil, "terminated pod3 should be nil")
+ assert.Equal(t, string(pods[3].UID), uid4, "pods with identical
creation time should be sorted deterministically by UID: expected pod4 at index
3")
assert.Assert(t, context.schedulerCache.GetPod(string(pod1.UID)) !=
nil, "expected to find pod 1 in cache")
assert.Assert(t, context.schedulerCache.GetPod(string(pod2.UID)) !=
nil, "expected to find pod 2 in cache")
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
