💬 ชาวเน็ตแห่แชร์: volcano sh/volcano/pull/

Tau721 · 2025-12-29T02:00:29Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Currently, the network-topology-aware plugin provides hypernode-level binpacking capability for jobs configured with a network topology. However, it fails to implement this feature for normal pods, i.e., the tasks of jobs that are not configured with any network topology. As a result, these tasks may be scheduled to all hypernodes in the cluster, leading to severe resource fragmentation and waste among hypernodes.

To this end, this PR tries to enhance the network-topology-aware plugin to provide hypernode-level binpacking for normal pods.

Which issue(s) this PR fixes:

Fixes #4869

Does this PR introduce a user-facing change?

The following two plugin arguments are added:
1. hypernode.binpack.normal-pod.enable: type of bool, used to control whether or not to enable hypernode-level binpacking for pods without network topology.
2. hypernode.binpack.normal-pod.fading: type of float64, used to control the tier weight of each hypernode, i.e., hypernode tier weight = math.Pow(fading, (tier-1)).

volcano-sh-bot · 2025-12-29T02:00:41Z

Welcome @Tau721! It looks like this is your first PR to volcano-sh/volcano 🎉

gemini-code-assist · 2025-12-29T02:00:47Z

Summary of Changes

Hello @Tau721, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly improves the network-topology-aware scheduler plugin by extending its hypernode-level binpacking capabilities. Previously, this feature was limited to jobs with explicit network topology configurations, leading to potential resource fragmentation for other pods. The changes introduce binpacking for these 'normal pods,' ensuring more efficient and consolidated resource utilization across hypernodes. This enhancement aims to reduce resource waste and optimize scheduling decisions for a broader range of workloads.

Highlights

Hypernode-level Binpacking for Normal Pods: The network-topology-aware plugin is enhanced to provide hypernode-level binpacking for pods that are not configured with any network topology, addressing resource fragmentation.
New Plugin Arguments: Two new plugin arguments are introduced: hypernode.binpack.normal-pod.enable (boolean) to control the feature, and hypernode.binpack.normal-pod.fading (float64) to adjust hypernode tier weights.
Resource Cache and Event Handling: A hyperNodeResourceCache is added to store resource status, and an EventHandler is implemented to update this cache on pod allocation and deallocation events, optimizing resource calculation.
Refactored Scoring Logic: The scoring functions (HyperNodeOrderFn, batchNodeOrderFn) have been refactored to separate logic for network-aware and normal pods, and a normalizeFinalScore function was introduced for consistent score scaling.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request enhances the network-topology-aware plugin to provide hypernode-level bin-packing for normal pods, which is a valuable feature for reducing resource fragmentation. The changes are well-structured, introducing new configuration options, refactoring the scoring logic for better clarity, and adding a resource cache to improve performance. The accompanying tests are comprehensive. I have a few suggestions, including a correction for a potential logic flaw in the new scoring mechanism that might impact scheduling behavior, a performance optimization for the resource cache updates, and a minor logging improvement.

gemini-code-assist · 2025-12-29T02:03:04Z

+	for _, node := range nodes {
+		totalScore := 0.0
+		for tier := nta.hyperNodesTier.minTier; tier <= nta.hyperNodesTier.maxTier; tier++ {
+			tierScore := FullScore


The initialization tierScore := FullScore seems logically incorrect for the goal of tiered bin-packing. When a node is not part of any hypernode in a given tier, it receives a perfect score (FullScore) for that tier. This rewards nodes for not being in a hypernode of a certain tier, which can lead to preferring nodes in higher-tiered hypernodes over nodes in lower-tiered ones, contradicting the goal of filling lower-tiered hypernodes first.

Consider changing the default score to ZeroScore to correctly penalize nodes that do not belong to a hypernode of a given tier. This would ensure that nodes within hypernodes of lower (more preferred) tiers are prioritized correctly.

Suggested change

tierScore := FullScore

tierScore := ZeroScore

When some node doesn't have a hypernode at the n-th tier, we can simply but equivalently consider that this node has a virtual hypernode at the n-th tier and its resources are fully utilized. Hence, we set its binpacking score to FullScore.

gemini-code-assist · 2025-12-29T02:03:04Z

 		}
 	}
+	msg = append(msg, fmt.Sprintf("%s[%t]", HyperNodeBinPackNormalPodEnable, n.normalPodConfig.hyperNodeBinPackingEnable),
+		fmt.Sprintf("%s[%f]", HyperNodeBinPackNormalPodFading, n.normalPodConfig.hyperNodeBinPackingFading))


Using %f for logging the hyperNodeBinPackingFading float value can result in unnecessary trailing zeros (e.g., 0.800000). Using %g would provide a more compact and readable representation (e.g., 0.8).

Suggested change

fmt.Sprintf("%s[%f]", HyperNodeBinPackNormalPodFading, n.normalPodConfig.hyperNodeBinPackingFading))

fmt.Sprintf("%s[%g]", HyperNodeBinPackNormalPodFading, n.normalPodConfig.hyperNodeBinPackingFading))

gemini-code-assist · 2025-12-29T02:03:04Z

+			for hyperNode := range ssn.HyperNodes {
+				if ssn.RealNodesSet[hyperNode].Has(node) {
+					if _, foundHyperNode := nta.hyperNodeResourceCache[hyperNode]; foundHyperNode {
+						for _, resource := range task.Resreq.ResourceNames() {
+							if _, foundResource := nta.hyperNodeResourceCache[hyperNode][resource]; foundResource {
+								nta.hyperNodeResourceCache[hyperNode][resource].used += task.Resreq.Get(resource)
+							}
+						}
+					}
+				}
+			}


The current implementation iterates over all hypernodes (for hyperNode := range ssn.HyperNodes) for each task allocation to update the resource cache. This can be inefficient if the number of hypernodes is large.

A more performant approach would be to directly find the hypernode(s) a node belongs to and update only those. This could be achieved by building a reverse mapping from a node to its ancestor hypernodes during session open.

For example:

// In OnSessionOpen, build a map like: // nodeToHyperNodes := map[string][]string // // Then in AllocateFunc: if ancestors, found := nodeToHyperNodes[node]; found { for _, hyperNode := range ancestors { // update cache for hyperNode } }

This would change the complexity from O(num_hypernodes) to O(depth_of_topology) for each allocation event.

Actually, the number of nodes is usually much larger than that of hypernodes, and the AllocateFunc or DeallocateFunc usually only uses the information of a few nodes. Therefore, caching the nodeToHypernodes map is considered not worth it.

wangyang0616 · 2025-12-29T02:41:51Z

Please supplement the corresponding capability scenarios and implementation descriptions in the design document.
Refer to Network Topology Aware Scheduling.md

ouyangshengjia · 2025-12-31T03:21:12Z

+	*normalPodConfig
 	*hyperNodesTier
+	// hyperNodeResourceCache stores the resource status of hypernodes to avoid repeated calculation: hypernode -> resource -> status
+	hyperNodeResourceCache map[string]map[corev1.ResourceName]*resourceStatus


Considering that this cache may be used frequently in future evolutions, it is recommended to fully initialize this cache in OnSessionOpen and replace the current lazy calculation implementation.

Perhaps issue #4899 could reuse this cache to pre-filtering HyperNodes during scheduiling.

I agree with you, and I have fixed it.

ouyangshengjia · 2026-01-07T02:49:11Z

+	hyperNodeResourceCache map[string]map[corev1.ResourceName]*resourceStatus
+}


Suggested change

hyperNodeResourceCache map[string]map[corev1.ResourceName]*resourceStatus

}

hyperNodeResourceStat map[string]resourceStat // hypernode -> resourceStat

}

type resourceStat struct {

used *api.Resource

allocatable *api.Resource

}

It is recommended to use the api.Resource struct for future expansion.

volcano/pkg/scheduler/api/resource_info.go

Lines 58 to 68 in 703a8f3

type Resource struct {

MilliCPU float64

Memory float64

// ScalarResources

ScalarResources map[v1.ResourceName]float64

// MaxTaskNum is only used by predicates; it should NOT

// be accounted in other operators, e.g. Add.

MaxTaskNum int

}

Thanks for your suggestion, and I have fixed it.

qi-min · 2026-01-08T13:38:05Z

+	args.GetFloat64(&config.hyperNodeBinPackingFading, HyperNodeBinPackNormalPodFading)
+	// config.hyperNodeBinPackingFading could be 0, which implies only the hypernodes of tier 1 affect the pod binpacking scores
+	if config.hyperNodeBinPackingFading < 0 {
+		config.hyperNodeBinPackingFading = 0.8


it would be better to use constant instead

Thanks for you review, and I have fixed it.

ouyangshengjia · 2026-01-13T03:56:55Z

+func (nta *networkTopologyAwarePlugin) normalizeFinalScore(scores map[string]float64) map[string]float64 {
+	normalizedScores := make(map[string]float64)
+	for name, score := range scores {
+		normalizedScores[name] = float64(k8sFramework.MaxNodeScore) * float64(nta.weight.GlobalWeight) * score
+	}
+	return normalizedScores
+}


I don't think the processing of this function can be called normalize.

How about changing it to scale?

ouyangshengjia · 2026-01-13T06:33:16Z

+	if totalTierWeight <= 0 {
+		// This should not happen, since there are at least one tier and its weight is one
+		klog.Warningf("the total tier weight of plugin %s should be greater than zero, but got %g", PluginName, totalTierWeight)
+		return nil, fmt.Errorf("the total tier weight of plugin %s should be greater than zero, but got %g", PluginName, totalTierWeight)


I think we can just leave a log here an return empty result. Considering that when we return an error in NodeOrderFn, the allocate process would be broken and the pod won't be scheduled.

I agree, and I have fixed it.

ouyangshengjia · 2026-01-13T06:39:19Z

+	for _, node := range nodes {
+		totalScore := 0.0
+		for tier := nta.hyperNodesTier.minTier; tier <= nta.hyperNodesTier.maxTier; tier++ {
+			tierScore := FullScore


It is recommended to add a comment here: Prefers to schedule pods on nodes that do not belong to any hypernode.

ouyangshengjia · 2026-01-13T06:49:11Z

/ok-to-test

JesseStutler · 2026-01-13T03:55:07Z

+	// HyperNodeBinPackNormalPodEnable is the key for whether to enable hypernode-level binpacking for pods without network topology
+	HyperNodeBinPackNormalPodEnable = "hypernode.binpack.normal-pod.enable"
+	// HyperNodeBinPackNormalPodFading is the key for tier weight fading parameter for pods without network topology
+	HyperNodeBinPackNormalPodFading = "hypernode.binpack.normal-pod.fading"


I think you should also update the design doc and user guide doc to teach users how to use it and what does they mean

Ok. I will try my best to finish it early.

ouyangshengjia · 2026-01-13T07:08:05Z

 		klog.V(5).Infof("Leaving networkTopologyAware plugin ...")
 	}()
 	nta.hyperNodesTier.init(ssn.HyperNodesTiers)
+	nta.initHyperNodeResourceCache(ssn)


The hyperNodeResourceCache is initialized each time a session is started. Is there any impact on a large cluster? Are there any performance test for this function?

Actually, I have evaluated the time efficiency of this initHyperNodeResourceCache function through the Test_initHyperNodeResourceCache function in the test file. The results demonstrate that aiming at a cluster with 2000 nodes, 200 hypernodes of tier 1, 40 hypernodes of tier 2, 5 hypernodes of tier 3, 1 hypernode of tier 4, and 3 resource types, it takes only about 3 milliseconds to initialize this hyperNodeResourceCache. Such time cost is totally acceptable.

Besides, I have also evaluated the time efficiency of the new batchNodeOrderFnForNormalPods function (which calculates the hypernode-level binpacking score for normal pods) through the Test_batchNodeOrderFnForNormalPods function in the test file. The results show that aiming at the same cluster, it takes about 10~15 milliseconds to invoke this function to score the whole 2000 nodes for one task. Such time cost is also acceptable.

ouyangshengjia · 2026-01-16T01:36:41Z

/lgtm
defers to @JesseStutler

JesseStutler · 2026-01-18T06:27:06Z

Waiting for @Tau721 #4889 (comment) finish the doc and then we can get merged

Signed-off-by: caotuo721 <caotuo721@yeah.net>

ouyangshengjia · 2026-01-20T07:24:03Z

/lgtm

JesseStutler

/approve

volcano-sh-bot · 2026-01-20T11:38:45Z

[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

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [JesseStutler]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

volcano-sh-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. labels Dec 29, 2025

volcano-sh-bot requested review from archlitchi and shinytang6 December 29, 2025 02:00

volcano-sh-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. do-not-merge/contains-merge-commits labels Dec 29, 2025

gemini-code-assist Bot reviewed Dec 29, 2025

View reviewed changes

ouyangshengjia reviewed Dec 31, 2025

View reviewed changes

Tau721 force-pushed the feat_hn-level_normal_pods_binpacking branch from 9715bad to 287704c Compare December 31, 2025 03:24

volcano-sh-bot removed the do-not-merge/contains-merge-commits label Dec 31, 2025

Tau721 force-pushed the feat_hn-level_normal_pods_binpacking branch 3 times, most recently from 2a0af7b to 9a8914b Compare January 3, 2026 14:42

Tau721 changed the title ~~[WIP]Enhance network-topology-aware plugin to provide hypernode-level binpacking for normal pods~~ Enhance network-topology-aware plugin to provide hypernode-level binpacking for normal pods Jan 6, 2026

volcano-sh-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 6, 2026

ouyangshengjia reviewed Jan 7, 2026

View reviewed changes

Tau721 force-pushed the feat_hn-level_normal_pods_binpacking branch from 746d048 to 1e59e7a Compare January 8, 2026 06:37

volcano-sh-bot added the do-not-merge/contains-merge-commits label Jan 8, 2026

Tau721 force-pushed the feat_hn-level_normal_pods_binpacking branch from adf7420 to 1de4f0c Compare January 8, 2026 06:58

volcano-sh-bot removed the do-not-merge/contains-merge-commits label Jan 8, 2026

Tau721 force-pushed the feat_hn-level_normal_pods_binpacking branch 2 times, most recently from 834591e to 2c697c0 Compare January 8, 2026 13:05

qi-min reviewed Jan 8, 2026

View reviewed changes

Tau721 requested review from ouyangshengjia and qi-min January 9, 2026 02:02

This was referenced Jan 13, 2026

perf(nta) NTA scheduling performance improvement by optimizing hyperNodeGradientFn #4939

Closed

perf(nta) NTA scheduling performance improvement by optimizing hyperNodeGradientFn #4940

Merged

ouyangshengjia reviewed Jan 13, 2026

View reviewed changes

volcano-sh-bot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jan 13, 2026

JesseStutler reviewed Jan 13, 2026

View reviewed changes

ouyangshengjia reviewed Jan 13, 2026

View reviewed changes

Tau721 force-pushed the feat_hn-level_normal_pods_binpacking branch 3 times, most recently from 35cc361 to 2283848 Compare January 15, 2026 13:47

volcano-sh-bot assigned ouyangshengjia Jan 16, 2026

volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 16, 2026

volcano-sh-bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 20, 2026

Tau721 force-pushed the feat_hn-level_normal_pods_binpacking branch 3 times, most recently from c6a1aa9 to 3aa0509 Compare January 20, 2026 05:31

Tau721 added 2 commits January 20, 2026 13:32

Support hypernode-level binpacking for normal pods

34ba695

Signed-off-by: caotuo721 <caotuo721@yeah.net>

Update the user guide doc and the design doc

9728115

Signed-off-by: caotuo721 <caotuo721@yeah.net>

Tau721 force-pushed the feat_hn-level_normal_pods_binpacking branch from 3aa0509 to 9728115 Compare January 20, 2026 05:32

volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 20, 2026

JesseStutler reviewed Jan 20, 2026

View reviewed changes

volcano-sh-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 20, 2026

volcano-sh-bot merged commit ad77883 into volcano-sh:master Jan 20, 2026
22 of 23 checks passed

Tau721 deleted the feat_hn-level_normal_pods_binpacking branch January 21, 2026 00:44

	fmt.Sprintf("%s[%f]", HyperNodeBinPackNormalPodFading, n.normalPodConfig.hyperNodeBinPackingFading))
	fmt.Sprintf("%s[%g]", HyperNodeBinPackNormalPodFading, n.normalPodConfig.hyperNodeBinPackingFading))

		hyperNodeResourceCache map[string]map[corev1.ResourceName]*resourceStatus
		}

-	hyperNodeResourceCache map[string]map[corev1.ResourceName]*resourceStatus
-}
+	hyperNodeResourceStat map[string]resourceStat // hypernode -> resourceStat
+}
+type resourceStat struct {
+	used *api.Resource
+	allocatable *api.Resource
+}

	type Resource struct {
	MilliCPU float64
	Memory float64

	// ScalarResources
	ScalarResources map[v1.ResourceName]float64

	// MaxTaskNum is only used by predicates; it should NOT
	// be accounted in other operators, e.g. Add.
	MaxTaskNum int
	}

Uh oh!

Conversation

Tau721 commented Dec 29, 2025

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Does this PR introduce a user-facing change?

Uh oh!

volcano-sh-bot commented Dec 29, 2025

Uh oh!

gemini-code-assist Bot commented Dec 29, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Tau721 Jan 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wangyang0616 commented Dec 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ouyangshengjia Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ouyangshengjia commented Jan 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Tau721 Jan 3, 2026 •

edited

Loading

ouyangshengjia Jan 7, 2026 •

edited

Loading

Tau721 Jan 13, 2026 •

edited

Loading