Recycle slice of evicted jobs in EvictJobsFromNode calls

armadaproject · Jun 29, 2023 · f394b24 · f394b24
1 parent 60944e7
commit f394b24
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 8 deletions.
diff --git a/internal/scheduler/nodedb/nodedb.go b/internal/scheduler/nodedb/nodedb.go
@@ -553,24 +553,28 @@ func bindJobToNodeInPlace(priorityClasses map[string]configuration.PriorityClass
 //   - Within AllocatableByPriorityAndResource, the resources allocated to these jobs are moved from
 //     the jobs' priorities to evictedPriority; they are not subtracted from AllocatedByJobId and
 //     AllocatedByQueue.
+//
+// The evictedJobs parameter is a pre-allocated slice for the evicted jobs; this function initially
+// clears it, and then appends all evicted jobs to it.
 func EvictJobsFromNode(
 	priorityClasses map[string]configuration.PriorityClass,
 	jobFilter func(interfaces.LegacySchedulerJob) bool,
+	evictedJobs []interfaces.LegacySchedulerJob,
 	jobs []interfaces.LegacySchedulerJob,
 	node *schedulerobjects.Node,
 ) ([]interfaces.LegacySchedulerJob, *schedulerobjects.Node, error) {
-	evicted := make([]interfaces.LegacySchedulerJob, 0)
+	evictedJobs = evictedJobs[0:0]
 	node = node.DeepCopy()
 	for _, job := range jobs {
 		if jobFilter != nil && !jobFilter(job) {
 			continue
 		}
-		evicted = append(evicted, job)
+		evictedJobs = append(evictedJobs, job)
 		if err := evictJobFromNodeInPlace(priorityClasses, job, node); err != nil {
 			return nil, nil, err
 		}
 	}
-	return evicted, node, nil
+	return evictedJobs, node, nil
 }
 
 // evictJobFromNodeInPlace is the in-place operation backing EvictJobsFromNode.

diff --git a/internal/scheduler/nodedb/nodedb_test.go b/internal/scheduler/nodedb/nodedb_test.go
@@ -131,7 +131,7 @@ func TestNodeBindingEvictionUnbinding(t *testing.T) {
 	unboundMultipleNode, err := UnbindJobsFromNode(testfixtures.TestPriorityClasses, []interfaces.LegacySchedulerJob{job}, boundNode)
 	require.NoError(t, err)
 
-	evictedJobs, evictedNode, err := EvictJobsFromNode(testfixtures.TestPriorityClasses, jobFilter, []interfaces.LegacySchedulerJob{job}, boundNode)
+	evictedJobs, evictedNode, err := EvictJobsFromNode(testfixtures.TestPriorityClasses, jobFilter, nil, []interfaces.LegacySchedulerJob{job}, boundNode)
 	require.NoError(t, err)
 	assert.Equal(t, []interfaces.LegacySchedulerJob{job}, evictedJobs)
 
@@ -141,7 +141,7 @@ func TestNodeBindingEvictionUnbinding(t *testing.T) {
 	evictedBoundNode, err := BindJobToNode(testfixtures.TestPriorityClasses, job, evictedNode)
 	require.NoError(t, err)
 
-	_, _, err = EvictJobsFromNode(testfixtures.TestPriorityClasses, jobFilter, []interfaces.LegacySchedulerJob{job}, node)
+	_, _, err = EvictJobsFromNode(testfixtures.TestPriorityClasses, jobFilter, nil, []interfaces.LegacySchedulerJob{job}, node)
 	require.Error(t, err)
 
 	_, err = UnbindJobFromNode(testfixtures.TestPriorityClasses, job, node)
@@ -150,7 +150,7 @@ func TestNodeBindingEvictionUnbinding(t *testing.T) {
 	_, err = BindJobToNode(testfixtures.TestPriorityClasses, job, boundNode)
 	require.Error(t, err)
 
-	_, _, err = EvictJobsFromNode(testfixtures.TestPriorityClasses, jobFilter, []interfaces.LegacySchedulerJob{job}, evictedNode)
+	_, _, err = EvictJobsFromNode(testfixtures.TestPriorityClasses, jobFilter, nil, []interfaces.LegacySchedulerJob{job}, evictedNode)
 	require.Error(t, err)
 
 	assertNodeAccountingEqual(t, node, unboundNode)
@@ -296,7 +296,7 @@ func TestEviction(t *testing.T) {
 				require.NoError(t, err)
 			}
 
-			actualEvictions, _, err := EvictJobsFromNode(testfixtures.TestPriorityClasses, tc.jobFilter, jobs, node)
+			actualEvictions, _, err := EvictJobsFromNode(testfixtures.TestPriorityClasses, tc.jobFilter, nil, jobs, node)
 			require.NoError(t, err)
 			expectedEvictions := make([]interfaces.LegacySchedulerJob, 0, len(tc.expectedEvictions))
 			for _, i := range tc.expectedEvictions {

diff --git a/internal/scheduler/preempting_queue_scheduler.go b/internal/scheduler/preempting_queue_scheduler.go
@@ -787,6 +787,7 @@ func NewOversubscribedEvictor(
 // Any job for which jobFilter returns true is evicted (if the node was not skipped).
 // If a job was evicted from a node, postEvictFunc is called with the corresponding job and node.
 func (evi *Evictor) Evict(ctx context.Context, it nodedb.NodeIterator) (*EvictorResult, error) {
+	evictedJobs := make([]interfaces.LegacySchedulerJob, 0)
 	var jobFilter func(job interfaces.LegacySchedulerJob) bool
 	if evi.jobFilter != nil {
 		jobFilter = func(job interfaces.LegacySchedulerJob) bool { return evi.jobFilter(ctx, job) }
@@ -808,7 +809,7 @@ func (evi *Evictor) Evict(ctx context.Context, it nodedb.NodeIterator) (*Evictor
 		if err != nil {
 			return nil, err
 		}
-		evictedJobs, node, err := nodedb.EvictJobsFromNode(evi.priorityClasses, jobFilter, jobs, node)
+		evictedJobs, node, err = nodedb.EvictJobsFromNode(evi.priorityClasses, jobFilter, evictedJobs, jobs, node)
 		if err != nil {
 			return nil, err
 		}