Repository: hashicorp/nomad
File: scheduler/generic_sched.go (Go)

Code before:
		return fmt.Errorf("failed to get tainted nodes for job '%s': %v",
			s.eval.JobID, err)
	}

	// Update the allocations which are in pending/running state on tainted
	// nodes to lost, but only if the scheduler has already marked them
	updateNonTerminalAllocsToLost(s.plan, tainted, allocs)

	r := reconciler.NewAllocReconciler(s.logger,
		genericAllocUpdateFn(s.ctx, s.stack, s.eval.ID),
		reconciler.ReconcilerState{
			Job:               s.job,
			JobID:             s.eval.JobID,
			JobIsBatch:        s.batch,
			DeploymentCurrent: s.deployment,
			ExistingAllocs:    allocs,
			EvalID:            s.eval.ID,
			EvalPriority:      s.eval.Priority,
		},
		reconciler.ClusterState{
			TaintedNodes:                tainted,
			SupportsDisconnectedClients: s.planner.ServersMeetMinimumVersion(minVersionMaxClientDisconnect, true),
			Now:                         time.Now().UTC(),
		})
	result := r.Compute()
	s.logger.Debug("reconciled current state with desired state", result.Fields()...)

	if s.eval.AnnotatePlan {
		s.plan.Annotations = &structs.PlanAnnotations{
			DesiredTGUpdates: result.DesiredTGUpdates,
		}
	}

	// Add the deployment changes to the plan
	s.plan.Deployment = result.Deployment
	s.plan.DeploymentUpdates = result.DeploymentUpdates

	// Store all the follow up evaluations from rescheduled allocations
	if len(result.DesiredFollowupEvals) > 0 {
		for _, evals := range result.DesiredFollowupEvals {
			s.followUpEvals = append(s.followUpEvals, evals...)
		}
	}

	// Update the stored deployment
	if result.Deployment != nil {
		s.deployment = result.Deployment
	}

	// Handle the stop
	for _, stop := range result.Stop {

Review comment (bug):
I think this was relevant before this change, but seeing the structuring method makes several len and string concatenation calls, I wondered if it was worth gating the log line using `IsDebug`, so we avoid this overhead is the log line isn't going to get emitted? No evidence for overhead though, just a thought.

Code after:
		return fmt.Errorf("failed to get tainted nodes for job '%s': %v",
			s.eval.JobID, err)
	}

	// Update the allocations which are in pending/running state on tainted
	// nodes to lost, but only if the scheduler has already marked them
	updateNonTerminalAllocsToLost(s.plan, tainted, allocs)

	r := reconciler.NewAllocReconciler(s.logger,
		genericAllocUpdateFn(s.ctx, s.stack, s.eval.ID),
		reconciler.ReconcilerState{
			Job:               s.job,
			JobID:             s.eval.JobID,
			JobIsBatch:        s.batch,
			DeploymentCurrent: s.deployment,
			ExistingAllocs:    allocs,
			EvalID:            s.eval.ID,
			EvalPriority:      s.eval.Priority,
		},
		reconciler.ClusterState{
			TaintedNodes:                tainted,
			SupportsDisconnectedClients: s.planner.ServersMeetMinimumVersion(minVersionMaxClientDisconnect, true),
			Now:                         time.Now().UTC(),
		})
	result := r.Compute()
	if s.logger.IsDebug() {
		s.logger.Debug("reconciled current state with desired state", result.Fields()...)
	}

	if s.eval.AnnotatePlan {
		s.plan.Annotations = &structs.PlanAnnotations{
			DesiredTGUpdates: result.DesiredTGUpdates,
		}
	}

	// Add the deployment changes to the plan
	s.plan.Deployment = result.Deployment
	s.plan.DeploymentUpdates = result.DeploymentUpdates

	// Store all the follow up evaluations from rescheduled allocations
	if len(result.DesiredFollowupEvals) > 0 {
		for _, evals := range result.DesiredFollowupEvals {
			s.followUpEvals = append(s.followUpEvals, evals...)
		}
	}

	// Update the stored deployment
	if result.Deployment != nil {
		s.deployment = result.Deployment
	}