Add network-topology-aware plugin and hyperNode score callback #3894

ecosysbin · 2024-12-16T03:16:12Z

Add network-topology-aware plugin and hyperNode score callback
Volcano scheduler: Supports network topology aware scheduling when pods rescheduled

volcano-sh-bot · 2024-12-16T03:16:15Z

Welcome @ecosysbin!

It looks like this is your first PR to volcano-sh/volcano.

Thank you, and welcome to Volcano. 😃

ecosysbin · 2024-12-16T07:41:57Z

/assign @Monokaix

JesseStutler · 2024-12-17T06:54:46Z

pkg/scheduler/api/job_info.go

+
+	// The RootHypernode property of a job is the hypernode that serves as the smallest root in the hypernode tree.
+	// A job has multiple tasks, each belonging to a hypernode. This RootHypernode is the topmost and lowest common ancestor among the hypernodes of all tasks within the job.
+	RootHyperNode string


When is RootHyperNode assigned? I didn't find any codes assign the value to RootHyperNode

when the task of job commit, the hyperNode of the task will assigned to job. it will add the code later.

JesseStutler · 2024-12-17T06:55:33Z

pkg/scheduler/api/types.go

@@ -300,6 +301,9 @@ type NodeReduceFn func(*TaskInfo, k8sframework.NodeScoreList) error
 // NodeOrderMapFn is the func declaration used to get priority score of all plugins for a node for a particular task.
 type NodeOrderMapFn func(*TaskInfo, *NodeInfo) (map[string]float64, float64, error)

+// HyperNodeOrderFn is the func declaration used to score hyperNodes for job.
+type HyperNodeOrderFn func(*JobInfo, map[string][]*NodeInfo, []int, map[int][]string, map[string]sets.Set[string]) (map[string]float64, error)


You may need to rebase this pr and factor your code:https://github.com/volcano-sh/volcano/pull/3874/files, the HyperNodeOrderFn structure has changed.

ok. l will do it later.

JesseStutler · 2024-12-17T06:56:56Z

pkg/scheduler/framework/session.go

+	// hyperNode can gain a better performance, the lower the tier of hyperNode, the better performance.
+	HyperNodesListByTier map[int][]string
+
+	HyperNodesTiers []int


Same as above, there are no so many fields now, may need to rebase and refactor: https://github.com/volcano-sh/volcano/pull/3874/files

ok. According to the discussion, there will have a hyperNode tree here. it will do in another PR.

JesseStutler · 2024-12-17T07:01:19Z

pkg/scheduler/plugins/networktopologyaware/networktopologyaware.go

+
+// Goals:
+// - The tier to which the rootHypernode of a job belongs should be as low as possible.
+func networkTopologyAwareScore(hyperNode string, job *api.JobInfo, hyperNodesTiers []int, hyperNodesListByTier map[int][]string, hyperNodesMap map[string]sets.Set[string]) float64 {


I didn’t get the point for this score func. Does this have a design plan or a picture? After refactoring based on this PR: https://github.com/volcano-sh/volcano/pull/3874/files, could you add more details or description about how to score hypernodes in your pr?

Explanation:
The RootHypernode property of a job is the hypernode that serves as the smallest root in the hypernode
tree.
A job has multiple tasks, each belonging to a hypernode. This RootHypernode is the topmost and lowest common ancestor among the hypernodes of all tasks within the job.

Goals:
The tier to which the rootHypernode of a job belongs should be as low as possible.

I will change the name 'RootHypernode' to 'LCAHyperNode' later.

JesseStutler · 2024-12-27T02:48:18Z

pkg/scheduler/plugins/networktopologyaware/networktopologyaware.go

+// - Tasks under a job should be scheduled to one hyperNode as much as possible.
+func networkTopologyAwareScore(hyperNodeName string, job *api.JobInfo, hyperNodeTree []map[string][]string) float64 {
+	jobHyperNode := job.PodGroup.GetAnnotations()[api.TopologyAllocateLCAHyperNode]
+	// job fist first scheduler.


-> //job is scheduled for the first time

JesseStutler · 2024-12-27T02:59:07Z

pkg/scheduler/plugins/networktopologyaware/networktopologyaware.go

+// HyperNodeTree is the hypernode tree of all hypernodes in the cluster.
+// currentJobLCAHyperNode is the hypernode of the job's LCAHyperNode.
+var (
+	HyperNodeTree []map[string][]string


Do we need to maintain a separate global variable here? And this data structure does not seem to be called HyperNodeTree, it does not reflect hierarchical relationships.

l have move the hyperNodeTree to session, According to the discussion, this variable will be replaced by Gu Peng's HyperNodeTree later.

JesseStutler · 2024-12-27T03:06:37Z

pkg/scheduler/util/scheduler_helper.go

+	return taskCount
+}
+
+// FindOutRootHyperNode find out the root hypernode of the job when the hypernode join the job.


-> //FindLCAHyperNode finds out the common ancestor of the current hypernode and the hypernode where the job is scheduled

JesseStutler · 2024-12-27T03:55:35Z

pkg/scheduler/util/scheduler_helper_test.go

+	}
+}
+
+func TestFindLCAHyperNode(t *testing.T) {


None of the use cases specify currentJobLCAHyperNode?

JesseStutler · 2024-12-27T06:18:58Z

pkg/scheduler/util/scheduler_helper.go

+	}
+
+	hyperNodesMap := make(map[string]sets.Set[string])
+	for i := 0; i < len(revertHyperNodeTree); i++ {


extract a public function, has the same code in FindLCAHyperNode

JesseStutler · 2024-12-27T06:27:08Z

pkg/scheduler/plugins/networktopologyaware/networktopologyaware.go

+	// PluginName indicates name of volcano scheduler plugin.
+	PluginName            = "networktopologyaware"
+	BaseScore             = 100
+	TaskBaseScore         = 10


Is 10 points reasonable? need to discuss /cc @Monokaix

To ensure that the scores for the task num are distributed between 0 and 10, so that the scores for the tier level will be higher than those for the task num.

JesseStutler · 2024-12-27T06:35:00Z

pkg/scheduler/framework/session.go

@@ -127,8 +126,10 @@ func openSession(cache cache.Cache) *Session {

 		TotalResource:  api.EmptyResource(),
 		TotalGuarantee: api.EmptyResource(),
-		podGroupStatus: map[api.JobID]scheduling.PodGroupStatus{},
-
+		PodGroupCache: api.PodGroupCache{


It is better to rename as PodGroupOldState, which is used to record some old PodGroup data.

JesseStutler · 2024-12-27T06:41:43Z

pkg/scheduler/cache/cache.go

 		job.PodGroup = pg
 	}
-
+	sc.Jobs[job.UID] = job


No need to update here

JesseStutler · 2024-12-27T06:44:25Z

pkg/scheduler/cache/cache.go

@@ -1536,9 +1537,12 @@ func (sc *SchedulerCache) UpdateJobStatus(job *schedulingapi.JobInfo, updatePG b
 		if err != nil {
 			return nil, err
 		}
+		sc.Mutex.Lock()


I think there is no need to add lock? session --> UpdateJobStatus is still in the same goroutine, and there is no other goroutine will read or write this map

According to the discussion, locks need to be added for all operations on the cache.

Here is to update podgroup to apiserver. If you just want to update cache, should not be here .

Yes, l need to update podgroup to apiserver.

JesseStutler · 2024-12-27T06:45:08Z