Fixing unfeasible scheduling keys bug (#182) (#3845)

* Fixing unfeasible scheduling keys bug * Renaming test parameter to be more consistent with other parameters * Renaming constraints Co-authored-by: Mustafa Ilyas <[email protected]>
armadaproject · Jul 29, 2024 · 8a64c0f · 8a64c0f
1 parent 29c293e
commit 8a64c0f
Show file tree

Hide file tree

Showing 4 changed files with 146 additions and 73 deletions.
diff --git a/internal/scheduler/constraints/constraints.go b/internal/scheduler/constraints/constraints.go
@@ -33,19 +33,22 @@ const (
 	GangExceedsGlobalBurstSizeUnschedulableReason = "gang cardinality too large: exceeds global max burst size"
 	GangExceedsQueueBurstSizeUnschedulableReason  = "gang cardinality too large: exceeds queue max burst size"
 
+	// Indicates that jobs cannot be scheduled due current executor state
+	GangDoesNotFitUnschedulableReason = "unable to schedule gang since minimum cardinality not met"
+	JobDoesNotFitUnschedulableReason  = "job does not fit on any node"
+
 	UnschedulableReasonMaximumResourcesExceeded = "resource limit exceeded"
 )
 
+func UnschedulableReasonIsPropertyOfGang(reason string) bool {
+	return reason == GangExceedsGlobalBurstSizeUnschedulableReason || reason == JobDoesNotFitUnschedulableReason || reason == GangDoesNotFitUnschedulableReason
+}
+
 // IsTerminalUnschedulableReason returns true if reason indicates
 // it's not possible to schedule any more jobs in this round.
 func IsTerminalUnschedulableReason(reason string) bool {
-	if reason == MaximumResourcesScheduledUnschedulableReason {
-		return true
-	}
-	if reason == GlobalRateLimitExceededUnschedulableReason {
-		return true
-	}
-	return false
+	return reason == MaximumResourcesScheduledUnschedulableReason ||
+		reason == GlobalRateLimitExceededUnschedulableReason
 }
 
 // IsTerminalQueueUnschedulableReason returns true if reason indicates

diff --git a/internal/scheduler/gang_scheduler.go b/internal/scheduler/gang_scheduler.go
@@ -82,11 +82,13 @@ func (sch *GangScheduler) updateGangSchedulingContextOnFailure(gctx *schedulerco
 		return err
 	}
 
-	// Register unfeasible scheduling keys.
+	globallyUnschedulable := schedulerconstraints.UnschedulableReasonIsPropertyOfGang(unschedulableReason)
+
+	// Register globally unfeasible scheduling keys.
 	//
 	// Only record unfeasible scheduling keys for single-job gangs.
 	// Since a gang may be unschedulable even if all its members are individually schedulable.
-	if !sch.skipUnsuccessfulSchedulingKeyCheck && gctx.Cardinality() == 1 {
+	if !sch.skipUnsuccessfulSchedulingKeyCheck && gctx.Cardinality() == 1 && globallyUnschedulable {
 		jctx := gctx.JobSchedulingContexts[0]
 		schedulingKey, ok := jctx.SchedulingKey()
 		if ok && schedulingKey != schedulerobjects.EmptySchedulingKey {
@@ -233,9 +235,9 @@ func (sch *GangScheduler) tryScheduleGangWithTxn(_ *armadacontext.Context, txn *
 	if ok, err = sch.nodeDb.ScheduleManyWithTxn(txn, gctx); err == nil {
 		if !ok {
 			if gctx.Cardinality() > 1 {
-				unschedulableReason = "unable to schedule gang since minimum cardinality not met"
+				unschedulableReason = schedulerconstraints.GangDoesNotFitUnschedulableReason
 			} else {
-				unschedulableReason = "job does not fit on any node"
+				unschedulableReason = schedulerconstraints.JobDoesNotFitUnschedulableReason
 			}
 		}
 		return