[release-1.15] fix(nodeorder): reuse session NodeMap and unify Score plugins through BatchNodeOrderFn - #5562
Conversation
The batch path used to build an fwk.NodeInfo per scored node via SetNode(node.Node), which leaves Allocatable/Requested unset. Any Score plugin that reads those fields would nil-panic if added to the batch path. This commit changes: 1. Pass ssn.NodeMap into BatchNodeOrderFn and look up the populated NodeInfo from it instead of constructing a new one. 2. Pass GetSnapshot().GetFwkNodeInfoMap() the same way in the agent scheduler nodeorder plugin. Signed-off-by: JesseStutler <chenzicong4@huawei.com>
pprof shows noderesources.Fit and BalancedAllocation recomputing the pod resource request on every per-node Score call, because the NodeOrderFn path skips PreScore. Plugins registered via the batch path go through nodescore.CalculatePluginScore, which runs PreScore once per batch and caches the parsed state. This commit changes: 1. Move Fit (Least/Most), BalancedAllocation and NodeAffinity from NodeOrderScorePlugins to ScorePlugins so they take the batch path. ImageLocality has no PreScore upstream and stays where it is. 2. Introduce scorePluginEntry to bundle plugin + normalizer + weight, and replace the per-plugin interPodAffinityScore / taintTolerationScore / podTopologySpreadScore wrappers with a single iteration over ScorePlugins. 3. Update TestInitPlugin to match the new plugin layout. Signed-off-by: JesseStutler <chenzicong4@huawei.com>
Signed-off-by: JesseStutler <chenzicong4@huawei.com>
volcano-sh-bot
added
the
size/L
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the node scoring mechanism in the Volcano scheduler by introducing a scorePluginEntry struct to encapsulate score plugins, their normalizers, and weights, and refactoring BatchNodeOrderFn to dynamically calculate scores. It also adds a helper function NodeInfosForCandidateNodes to reuse candidate node information, optimizing snapshot usage. The review feedback suggests defensive programming improvements to prevent potential nil pointer dereferences, specifically checking for nil nodeInfo in the helper function and falling back to an empty normalizer if a plugin's normalizer is nil.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if nodeInfo, ok := nodeMap[node.Name]; ok { | ||
| nodeInfos = append(nodeInfos, nodeInfo) | ||
| } |
There was a problem hiding this comment.
To prevent potential nil pointer or nil interface dereference panics in downstream score plugins, we should defensively check if the retrieved nodeInfo interface is non-nil before appending it to the nodeInfos slice.
| if nodeInfo, ok := nodeMap[node.Name]; ok { | |
| nodeInfos = append(nodeInfos, nodeInfo) | |
| } | |
| if nodeInfo, ok := nodeMap[node.Name]; ok && nodeInfo != nil { | |
| nodeInfos = append(nodeInfos, nodeInfo) | |
| } |
| for name, entry := range pp.ScorePlugins { | ||
| scores, err := nodescore.CalculatePluginScore(name, entry.plugin, entry.normalizer, state, task.Pod, nodeInfos, entry.weight) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
If a score plugin is registered in pp.ScorePlugins but its normalizer field is left uninitialized (i.e., nil), calling CalculatePluginScore will result in a nil pointer dereference panic when NormalizeScore is invoked. We should defensively fall back to &nodescore.EmptyNormalizer{} if entry.normalizer is nil.
for name, entry := range pp.ScorePlugins {
normalizer := entry.normalizer
if normalizer == nil {
normalizer = &nodescore.EmptyNormalizer{}
}
scores, err := nodescore.CalculatePluginScore(name, entry.plugin, normalizer, state, task.Pod, nodeInfos, entry.weight)
if err != nil {
return nil, err
}
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JesseStutler The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
volcano-sh-bot
added
the
approved
69368d1
into
volcano-sh:release-1.15
2 participants
This is an automated cherry-pick of #5497