This is an automated email from the ASF dual-hosted git repository.
HuangTing-Yao 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 4b35f0d3 [YUNIKORN-3379] Fix scheduler panic on PLACEHOLDER_REPLACED
release without linked replacement (#1150)
4b35f0d3 is described below
commit 4b35f0d39d8424a0415431c1e05d1b32dc827f48
Author: hedger9487 <[email protected]>
AuthorDate: Mon Sep 7 20:58:39 2026 +0800
[YUNIKORN-3379] Fix scheduler panic on PLACEHOLDER_REPLACED release without
linked replacement (#1150)
When an AllocationRelease of type PLACEHOLDER_REPLACED is processed for a
placeholder that has no linked replacement allocation (ph.GetRelease() ==
nil),
replacePlaceholderOnNode dereferences confirmed.GetAllocatedResource()
without
a nil check, causing a SIGSEGV panic and taking down the scheduler.
This fix:
1. Adds a defensive nil check for confirmed in replacePlaceholderOnNode.
2. Emits a descriptive WARN log and cleans up the placeholder from the node
via
node.RemoveAllocation(), accumulating the freed resource into total so
queue
allocated resources are correctly decremented.
3. Safely returns nil to caller, allowing removeAllocation to decrement the
cluster
allocation count and notify the RM of the released placeholder.
4. Adds TestRemoveAllocationPlaceholderReplacedWithoutReplacement in
partition_test.go,
reproducing the defect and asserting that node, queue, and partition
states are
cleanly restored.
Closes: #1150
Signed-off-by: HuangTing-Yao <[email protected]>
---
pkg/scheduler/partition.go | 9 +++++++++
pkg/scheduler/partition_test.go | 40 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 49 insertions(+)
diff --git a/pkg/scheduler/partition.go b/pkg/scheduler/partition.go
index 3c3c9f20..c162e530 100644
--- a/pkg/scheduler/partition.go
+++ b/pkg/scheduler/partition.go
@@ -1598,6 +1598,15 @@ func (pc *PartitionContext) removeAllocation(release
*si.AllocationRelease) ([]*
// real allocation.
func (pc *PartitionContext) replacePlaceholderOnNode(alloc
*objects.Allocation, node *objects.Node, total *resources.Resource)
*objects.Allocation {
confirmed := alloc.GetRelease()
+ if confirmed == nil {
+ log.Log(log.SchedPartition).Warn("unexpected placeholder
replacement without replacement allocation, removing placeholder from node",
+ zap.String("nodeID", alloc.GetNodeID()),
+ zap.String("allocationKey", alloc.GetAllocationKey()))
+ if node.RemoveAllocation(alloc.GetAllocationKey()) != nil {
+ total.AddTo(alloc.GetAllocatedResource())
+ }
+ return nil
+ }
// we need to check the resources equality
delta := resources.Sub(confirmed.GetAllocatedResource(),
alloc.GetAllocatedResource())
// Any negative value in the delta means that at least one of the
requested resource in the
diff --git a/pkg/scheduler/partition_test.go b/pkg/scheduler/partition_test.go
index eb67e11d..40dcfbd1 100644
--- a/pkg/scheduler/partition_test.go
+++ b/pkg/scheduler/partition_test.go
@@ -5718,3 +5718,43 @@ func TestRemoveAppWithReservations(t *testing.T) {
partition.removeApplication(appID1)
assert.Equal(t, 0, partition.getReservationCount())
}
+
+func TestRemoveAllocationPlaceholderReplacedWithoutReplacement(t *testing.T) {
+ setupUGM()
+ partition, err := newBasePartition()
+ assert.NilError(t, err, "partition create failed")
+ defer partition.userGroupCache.Stop()
+
+ // add a new app
+ app := newApplication(appID1, "default", defQueue)
+ err = partition.AddApplication(app)
+ assert.NilError(t, err, "add application to partition should not have
failed")
+
+ // add a node with placeholder allocation
+ nodeRes :=
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10})
+ node1 := newNodeMaxResource(nodeID1, nodeRes)
+ appRes :=
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 1})
+ ph := newAllocationTG("placeholder", appID1, nodeID1, taskGroup,
appRes, true)
+ err = partition.AddNode(node1)
+ assert.NilError(t, err)
+ _, allocCreated, err := partition.UpdateAllocation(ph)
+ assert.NilError(t, err)
+ assert.Check(t, allocCreated)
+ assert.Equal(t, 1, partition.GetTotalAllocationCount())
+ assert.Assert(t,
resources.Equals(partition.GetQueue(defQueue).GetAllocatedResource(), appRes))
+
+ // Release with PLACEHOLDER_REPLACED when no replacement was ever
linked (ph.GetRelease() == nil)
+ release := &si.AllocationRelease{
+ PartitionName: partition.Name,
+ ApplicationID: appID1,
+ AllocationKey: "placeholder",
+ TerminationType: si.TerminationType_PLACEHOLDER_REPLACED,
+ }
+
+ released, confirmed := partition.removeAllocation(release)
+ assert.Assert(t, confirmed == nil, "confirmed allocation should be nil
when no replacement exists")
+ assert.Equal(t, 1, len(released), "placeholder should be in released
list")
+ assert.Equal(t, 0, partition.GetTotalAllocationCount(), "allocation
count should be 0 after placeholder removed")
+ assert.Assert(t, node1.GetAllocation("placeholder") == nil,
"placeholder should be removed from node")
+ assert.Assert(t,
resources.IsZero(partition.GetQueue(defQueue).GetAllocatedResource()), "queue
resource should be zero after placeholder removed")
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
