Address some code review comments

nomad/eval_endpoint_test.go

@@ -287,7 +287,7 @@ func TestEvalEndpoint_Dequeue_WaitIndex(t *testing.T) {
 }
 
 func TestEvalEndpoint_Dequeue_UpdateWaitIndex(t *testing.T) {
-	// test enqueing an eval, updating a plan result for the same eval and dequeing the eval
+	// test enqueueing an eval, updating a plan result for the same eval and de-queueing the eval
 	t.Parallel()
 	s1 := testServer(t, func(c *Config) {
 		c.NumSchedulers = 0 // Prevent automatic dequeue

nomad/state/state_store.go

@@ -1505,7 +1505,6 @@ func (s *StateStore) updateEvalModifyIndex(txn *memdb.Txn, index uint64, evalID
 	}
 	eval := existing.(*structs.Evaluation).Copy()
 	// Update the indexes
-	eval.CreateIndex = existing.(*structs.Evaluation).CreateIndex
 	eval.ModifyIndex = index
 
 	// Insert the eval

nomad/state/state_store_test.go

@@ -127,7 +127,7 @@ func TestStateStore_UpsertPlanResults_AllocationsCreated_Denormalized(t *testing
 
 	index, err := state.Index("allocs")
 	assert.Nil(err)
-	assert.Equal(uint64(1000), index)
+	assert.EqualValues(1000, index)
 
 	if watchFired(ws) {
 		t.Fatalf("bad")
@@ -136,7 +136,7 @@ func TestStateStore_UpsertPlanResults_AllocationsCreated_Denormalized(t *testing
 	evalOut, err := state.EvalByID(ws, eval.ID)
 	assert.Nil(err)
 	assert.NotNil(evalOut)
-	assert.Equal(uint64(1000), evalOut.ModifyIndex)
+	assert.EqualValues(1000, evalOut.ModifyIndex)
 }
 
 // This test checks that the deployment is created and allocations count towards
@@ -198,7 +198,7 @@ func TestStateStore_UpsertPlanResults_Deployment(t *testing.T) {
 	evalOut, err := state.EvalByID(ws, eval.ID)
 	assert.Nil(err)
 	assert.NotNil(evalOut)
-	assert.Equal(uint64(1000), evalOut.ModifyIndex)
+	assert.EqualValues(1000, evalOut.ModifyIndex)
 
 	if watchFired(ws) {
 		t.Fatalf("bad")
@@ -240,7 +240,7 @@ func TestStateStore_UpsertPlanResults_Deployment(t *testing.T) {
 	evalOut, err = state.EvalByID(ws, eval.ID)
 	assert.Nil(err)
 	assert.NotNil(evalOut)
-	assert.Equal(uint64(1001), evalOut.ModifyIndex)
+	assert.EqualValues(1001, evalOut.ModifyIndex)
 }
 
 // This test checks that deployment updates are applied correctly
@@ -315,12 +315,12 @@ func TestStateStore_UpsertPlanResults_DeploymentUpdates(t *testing.T) {
 	assert.NotNil(doutstandingout)
 	assert.Equal(update.Status, doutstandingout.Status)
 	assert.Equal(update.StatusDescription, doutstandingout.StatusDescription)
-	assert.Equal(uint64(1000), doutstandingout.ModifyIndex)
+	assert.EqualValues(1000, doutstandingout.ModifyIndex)
 
 	evalOut, err := state.EvalByID(ws, eval.ID)
 	assert.Nil(err)
 	assert.NotNil(evalOut)
-	assert.Equal(uint64(1000), evalOut.ModifyIndex)
+	assert.EqualValues(1000, evalOut.ModifyIndex)
 	if watchFired(ws) {
 		t.Fatalf("bad")
 	}

nomad/structs/structs.go

@@ -515,10 +515,12 @@ type ApplyPlanResultsRequest struct {
 	// because the job is stopped or the update block is removed.
 	DeploymentUpdates []*DeploymentStatusUpdate
 
-	// EvalID is the eval ID of the plan being applied. We also update the modify
-	// index of the eval ID as part of applying plan results. This is to ensure that
-	// other workers that are dequeing evaluations don't miss updates that can affect
-	// scheduling decisions.
+	// EvalID is the eval ID of the plan being applied. The modify index of the
+	// evaluation is updated as part of applying the plan to ensure that subsequent
+	// scheduling events for the same job will wait for the index that last produced
+	// state changes. This is necessary for blocked evaluations since they can be
+	// processed many times, potentially making state updates, without the state of
+	// the evaluation itself being updated.
 	EvalID string
 }