Prevent urgency preemption from overriding fair-share evictions

internal/scheduler/nodedb/nodedb.go

@@ -641,35 +641,35 @@ func (nodeDb *NodeDb) selectNodeForJobWithTxnAtPriority(
 	pctx.NodeId = ""
 	pctx.PreemptedAtPriority = internaltypes.MinPriority
 
-	// Schedule by preventing evicted jobs from being re-scheduled.
-	// This method respect fairness by preventing from re-scheduling jobs that appear as far back in the total order as possible.
-	if !nodeDb.disableFairshareScheduling {
-		if node, err := nodeDb.selectNodeForJobWithFairPreemption(txn, jctx); err != nil {
+	// Schedule by kicking off jobs currently bound to a node.
+	// This method does not respect fairness when choosing on which node to schedule the job.
+	if !nodeDb.disableUrgencyScheduling {
+		if node, err := nodeDb.selectNodeForJobWithUrgencyPreemption(txn, jctx, matchingNodeTypeIds); err != nil {
 			return nil, err
 		} else if err := assertPodSchedulingContextNode(pctx, node); err != nil {
 			return nil, err
 		} else if node != nil {
-			pctx.SchedulingMethod = context.ScheduledWithFairSharePreemption
+			pctx.SchedulingMethod = context.ScheduledWithUrgencyBasedPreemption
 			return node, nil
 		}
 	}
 
-	pctx.NodeId = ""
-	pctx.PreemptedAtPriority = internaltypes.MinPriority
-
-	// Schedule by kicking off jobs currently bound to a node.
-	// This method does not respect fairness when choosing on which node to schedule the job.
-	if !nodeDb.disableUrgencyScheduling {
-		if node, err := nodeDb.selectNodeForJobWithUrgencyPreemption(txn, jctx, matchingNodeTypeIds); err != nil {
+	// Schedule by preventing evicted jobs from being re-scheduled.
+	// This method respect fairness by preventing from re-scheduling jobs that appear as far back in the total order as possible.
+	if !nodeDb.disableFairshareScheduling {
+		if node, err := nodeDb.selectNodeForJobWithFairPreemption(txn, jctx); err != nil {
 			return nil, err
 		} else if err := assertPodSchedulingContextNode(pctx, node); err != nil {
 			return nil, err
 		} else if node != nil {
-			pctx.SchedulingMethod = context.ScheduledWithUrgencyBasedPreemption
+			pctx.SchedulingMethod = context.ScheduledWithFairSharePreemption
 			return node, nil
 		}
 	}
 
+	pctx.NodeId = ""
+	pctx.PreemptedAtPriority = internaltypes.MinPriority
+
 	return nil, nil
 }
 

internal/scheduler/nodedb/nodedb_test.go

@@ -933,11 +933,6 @@ func TestPreemptionScheduling(t *testing.T) {
 			disableUrgencyScheduling: true,
 			expectSuccess:            false,
 		},
-		"fair-share preemption by default": {
-			registerEvictedJobs:      true,
-			expectSuccess:            true,
-			expectedSchedulingMethod: context.ScheduledWithFairSharePreemption,
-		},
 		"falls through to urgency-based preemption when fair-share disabled": {
 			registerEvictedJobs:        true,
 			disableFairshareScheduling: true,

internal/scheduler/scheduling/preempting_queue_scheduler_test.go

@@ -716,7 +716,7 @@ func TestPreemptingQueueScheduler(t *testing.T) {
 					// Schedule jobs that requires preempting one job in the gang,
 					// and assert that all jobs in the gang are preempted.
 					JobsByQueue: map[string][]*jobdb.Job{
-						"A": testfixtures.N1Cpu4GiJobs("A", testfixtures.PriorityClass1, 17),
+						"A": testfixtures.N1Cpu4GiJobs("A", testfixtures.PriorityClass0, 17),
 					},
 					ExpectedScheduledIndices: map[string][]int{
 						"A": testfixtures.IntRange(0, 16),