From 355d15c587e6a9679aedd909d9d436da5c5a3ded Mon Sep 17 00:00:00 2001
From: Chris Martin <council_tax@hotmail.com>
Date: Fri, 16 Feb 2024 08:45:12 +0000
Subject: [PATCH] Remove UnifiedSchedulingByPool Config Flag (#3402)

* remove non-unified pool scheduling

Signed-off-by: Chris Martin <chris@cmartinit.co.uk>

* put back test

Signed-off-by: Chris Martin <chris@cmartinit.co.uk>

---------

Signed-off-by: Chris Martin <chris@cmartinit.co.uk>
Co-authored-by: Chris Martin <chris@cmartinit.co.uk>
---
 internal/armada/configuration/types.go        |  5 +----
 internal/scheduler/scheduling_algo.go         | 21 ++++++-------------
 internal/scheduler/scheduling_algo_test.go    | 14 ++-----------
 .../scheduler/testfixtures/testfixtures.go    |  5 -----
 4 files changed, 9 insertions(+), 36 deletions(-)

diff --git a/internal/armada/configuration/types.go b/internal/armada/configuration/types.go
index a5ea6272f45..2d2c51cf767 100644
--- a/internal/armada/configuration/types.go
+++ b/internal/armada/configuration/types.go
@@ -85,10 +85,7 @@ type SchedulingConfig struct {
 	DisableScheduling bool
 	// Set to true to enable scheduler assertions. This results in some performance loss.
 	EnableAssertions bool
-	// If true, schedule jobs across all executors in the same pool in a unified manner.
-	// Otherwise, schedule each executor separately.
-	UnifiedSchedulingByPool bool
-	Preemption              PreemptionConfig
+	Preemption       PreemptionConfig
 	// Number of jobs to load from the database at a time.
 	MaxQueueLookback uint
 	// In each invocation of the scheduler, no more jobs are scheduled once this limit has been exceeded.
diff --git a/internal/scheduler/scheduling_algo.go b/internal/scheduler/scheduling_algo.go
index 4e136f358d7..374529c4cc2 100644
--- a/internal/scheduler/scheduling_algo.go
+++ b/internal/scheduler/scheduling_algo.go
@@ -200,21 +200,12 @@ func (l *FairSchedulingAlgo) Schedule(
 }
 
 func (l *FairSchedulingAlgo) groupExecutors(executors []*schedulerobjects.Executor) map[string][]*schedulerobjects.Executor {
-	if l.schedulingConfig.UnifiedSchedulingByPool {
-		return armadaslices.GroupByFunc(
-			executors,
-			func(executor *schedulerobjects.Executor) string {
-				return executor.Pool
-			},
-		)
-	} else {
-		return armadaslices.GroupByFunc(
-			executors,
-			func(executor *schedulerobjects.Executor) string {
-				return executor.Id
-			},
-		)
-	}
+	return armadaslices.GroupByFunc(
+		executors,
+		func(executor *schedulerobjects.Executor) string {
+			return executor.Pool
+		},
+	)
 }
 
 type JobQueueIteratorAdapter struct {
diff --git a/internal/scheduler/scheduling_algo_test.go b/internal/scheduler/scheduling_algo_test.go
index 5af4ce47ed8..922cf94f333 100644
--- a/internal/scheduler/scheduling_algo_test.go
+++ b/internal/scheduler/scheduling_algo_test.go
@@ -338,18 +338,8 @@ func TestSchedule(t *testing.T) {
 			},
 			expectedScheduledIndices: []int{0},
 		},
-		"UnifiedSchedulingByPool": {
-			schedulingConfig: testfixtures.WithUnifiedSchedulingByPoolConfig(testfixtures.TestSchedulingConfig()),
-			executors: []*schedulerobjects.Executor{
-				testfixtures.Test1Node32CoreExecutor("executor1"),
-				testfixtures.Test1Node32CoreExecutor("executor2"),
-			},
-			queues:                   []*database.Queue{testfixtures.TestDbQueue()},
-			queuedJobs:               testfixtures.N16Cpu128GiJobs(testfixtures.TestQueue, testfixtures.PriorityClass3, 10),
-			expectedScheduledIndices: []int{0, 1, 2, 3},
-		},
-		"UnifiedSchedulingByPool schedule gang job over multiple executors": {
-			schedulingConfig: testfixtures.WithUnifiedSchedulingByPoolConfig(testfixtures.TestSchedulingConfig()),
+		"Schedule gang job over multiple executors": {
+			schedulingConfig: testfixtures.TestSchedulingConfig(),
 			executors: []*schedulerobjects.Executor{
 				testfixtures.Test1Node32CoreExecutor("executor1"),
 				testfixtures.Test1Node32CoreExecutor("executor2"),
diff --git a/internal/scheduler/testfixtures/testfixtures.go b/internal/scheduler/testfixtures/testfixtures.go
index 0a1cd1c8601..70917ba1dfb 100644
--- a/internal/scheduler/testfixtures/testfixtures.go
+++ b/internal/scheduler/testfixtures/testfixtures.go
@@ -147,11 +147,6 @@ func TestSchedulingConfig() configuration.SchedulingConfig {
 	}
 }
 
-func WithUnifiedSchedulingByPoolConfig(config configuration.SchedulingConfig) configuration.SchedulingConfig {
-	config.UnifiedSchedulingByPool = true
-	return config
-}
-
 func WithMaxUnacknowledgedJobsPerExecutorConfig(v uint, config configuration.SchedulingConfig) configuration.SchedulingConfig {
 	config.MaxUnacknowledgedJobsPerExecutor = v
 	return config