From cde561a6366ee9bedfb5c1b7dc4c993b3402ecb9 Mon Sep 17 00:00:00 2001 From: Drew Erny Date: Tue, 16 Jul 2019 12:26:54 -0500 Subject: [PATCH] Fix update out of sequence A simple but old error has recently become evident. Due to the fact that we read an object and then write it back across the boundaries of a transaction, it is possible for the task object to have changed in between transactions. This would cause the attempt to write out the old task to suffer an "Update out of sequence" error. This fix simply reads the latest version of the task back out within the boundary of a transaction to avoid the race. Signed-off-by: Drew Erny (cherry picked from commit d68ac46e3b11d7384472677d210bb0ce941284dc) Signed-off-by: Sebastiaan van Stijn --- manager/orchestrator/service.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/manager/orchestrator/service.go b/manager/orchestrator/service.go index 037e493b30..c5d298c516 100644 --- a/manager/orchestrator/service.go +++ b/manager/orchestrator/service.go @@ -47,22 +47,27 @@ func SetServiceTasksRemove(ctx context.Context, s *store.MemoryStore, service *a err = s.Batch(func(batch *store.Batch) error { for _, t := range tasks { err := batch.Update(func(tx store.Tx) error { + // the task may have changed for some reason in the meantime + // since we read it out, so we need to get from the store again + // within the boundaries of a transaction + latestTask := store.GetTask(tx, t.ID) + // time travel is not allowed. if the current desired state is // above the one we're trying to go to we can't go backwards. // we have nothing to do and we should skip to the next task - if t.DesiredState > api.TaskStateRemove { + if latestTask.DesiredState > api.TaskStateRemove { // log a warning, though. we shouln't be trying to rewrite // a state to an earlier state log.G(ctx).Warnf( "cannot update task %v in desired state %v to an earlier desired state %v", - t.ID, t.DesiredState, api.TaskStateRemove, + latestTask.ID, latestTask.DesiredState, api.TaskStateRemove, ) return nil } // update desired state to REMOVE - t.DesiredState = api.TaskStateRemove + latestTask.DesiredState = api.TaskStateRemove - if err := store.UpdateTask(tx, t); err != nil { + if err := store.UpdateTask(tx, latestTask); err != nil { log.G(ctx).WithError(err).Errorf("failed transaction: update task desired state to REMOVE") } return nil