From 022b6dd5287534eee13c2a64c6c0cdb9ff22e2c3 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Fri, 4 Mar 2016 16:17:14 -0800 Subject: [PATCH 1/2] Fix bug introduced with paused nack timers --- client/driver/rkt_test.go | 26 +++++++++++++------------- nomad/eval_broker.go | 5 ++++- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/client/driver/rkt_test.go b/client/driver/rkt_test.go index c2bbd600c..76f2bce09 100644 --- a/client/driver/rkt_test.go +++ b/client/driver/rkt_test.go @@ -207,11 +207,11 @@ func TestRktDriver_Start_Wait_AllocDir(t *testing.T) { task := &structs.Task{ Name: "alpine", Config: map[string]interface{}{ - "image": "docker://alpine", - "command": "/bin/sh", - "args": []string{ - "-c", - fmt.Sprintf(`echo -n %s > ${%s}/%s`, string(exp), env.AllocDir, file), + "image": "docker://alpine", + "command": "/bin/sh", + "args": []string{ + "-c", + fmt.Sprintf(`echo -n %s > ${%s}/%s`, string(exp), env.AllocDir, file), }, }, LogConfig: &structs.LogConfig{ @@ -247,13 +247,13 @@ func TestRktDriver_Start_Wait_AllocDir(t *testing.T) { } // Check that data was written to the shared alloc directory. - outputFile := filepath.Join(execCtx.AllocDir.SharedDir, file) - act, err := ioutil.ReadFile(outputFile) - if err != nil { - t.Fatalf("Couldn't read expected output: %v", err) - } + outputFile := filepath.Join(execCtx.AllocDir.SharedDir, file) + act, err := ioutil.ReadFile(outputFile) + if err != nil { + t.Fatalf("Couldn't read expected output: %v", err) + } - if !reflect.DeepEqual(act, exp) { - t.Fatalf("Command output is %v; expected %v", act, exp) - } + if !reflect.DeepEqual(act, exp) { + t.Fatalf("Command output is %v; expected %v", act, exp) + } } diff --git a/nomad/eval_broker.go b/nomad/eval_broker.go index 6e5a3ed59..be4925dbb 100644 --- a/nomad/eval_broker.go +++ b/nomad/eval_broker.go @@ -78,6 +78,7 @@ type unackEval struct { Eval *structs.Evaluation Token string NackTimer *time.Timer + Paused bool } // PendingEvaluations is a list of waiting evaluations. @@ -419,7 +420,7 @@ func (b *EvalBroker) OutstandingReset(evalID, token string) error { if unack.Token != token { return ErrTokenMismatch } - if !unack.NackTimer.Reset(b.nackTimeout) { + if !unack.Paused && !unack.NackTimer.Reset(b.nackTimeout) { return ErrNackTimeoutReached } return nil @@ -525,6 +526,7 @@ func (b *EvalBroker) PauseNackTimeout(evalID, token string) error { if !unack.NackTimer.Stop() { return ErrNackTimeoutReached } + unack.Paused = true return nil } @@ -541,6 +543,7 @@ func (b *EvalBroker) ResumeNackTimeout(evalID, token string) error { return ErrTokenMismatch } unack.NackTimer.Reset(b.nackTimeout) + unack.Paused = false return nil } From 5cc91ecd7978768e42407cd7cac30d665a755714 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Fri, 4 Mar 2016 17:25:37 -0800 Subject: [PATCH 2/2] Remove outstanding reset from plan applier --- nomad/eval_broker.go | 5 +---- nomad/plan_apply.go | 8 -------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/nomad/eval_broker.go b/nomad/eval_broker.go index be4925dbb..6e5a3ed59 100644 --- a/nomad/eval_broker.go +++ b/nomad/eval_broker.go @@ -78,7 +78,6 @@ type unackEval struct { Eval *structs.Evaluation Token string NackTimer *time.Timer - Paused bool } // PendingEvaluations is a list of waiting evaluations. @@ -420,7 +419,7 @@ func (b *EvalBroker) OutstandingReset(evalID, token string) error { if unack.Token != token { return ErrTokenMismatch } - if !unack.Paused && !unack.NackTimer.Reset(b.nackTimeout) { + if !unack.NackTimer.Reset(b.nackTimeout) { return ErrNackTimeoutReached } return nil @@ -526,7 +525,6 @@ func (b *EvalBroker) PauseNackTimeout(evalID, token string) error { if !unack.NackTimer.Stop() { return ErrNackTimeoutReached } - unack.Paused = true return nil } @@ -543,7 +541,6 @@ func (b *EvalBroker) ResumeNackTimeout(evalID, token string) error { return ErrTokenMismatch } unack.NackTimer.Reset(b.nackTimeout) - unack.Paused = false return nil } diff --git a/nomad/plan_apply.go b/nomad/plan_apply.go index 52c575b14..5dc668f99 100644 --- a/nomad/plan_apply.go +++ b/nomad/plan_apply.go @@ -59,14 +59,6 @@ func (s *Server) planApply() { return } - // Verify the evaluation is outstanding, and that the tokens match. - if err := s.evalBroker.OutstandingReset(pending.plan.EvalID, pending.plan.EvalToken); err != nil { - s.logger.Printf("[ERR] nomad: plan rejected for evaluation %s: %v", - pending.plan.EvalID, err) - pending.respond(nil, err) - continue - } - // Check if out last plan has completed select { case <-waitCh: