From baa33d73265bbf18ad6a2dcdf441e73c3b21c6ea Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Fri, 7 Aug 2015 11:20:13 -0500 Subject: [PATCH] core: dag errors should cascade to all descendents We weren't marking skipped nodes as failing, so any grandchild-and-deeper dependencies would still evaluate. For example: A -> B -> C -> D If B failed, C would be skipped, but D would still be evaluated. This fixes the behavior so C, D, and any further descendents will all be skipped when B fails. Addresses crashing aspect of #2955 and likely a lot of other confusing failure modes. --- dag/dag.go | 5 +++-- dag/dag_test.go | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/dag/dag.go b/dag/dag.go index 0148ee07ca13..602cacd99927 100644 --- a/dag/dag.go +++ b/dag/dag.go @@ -195,7 +195,7 @@ func (g *AcyclicGraph) Walk(cb WalkFunc) error { // Start the goroutine to wait for our dependencies readyCh := make(chan bool) - go func(deps []Vertex, chs []<-chan struct{}, readyCh chan<- bool) { + go func(v Vertex, deps []Vertex, chs []<-chan struct{}, readyCh chan<- bool) { // First wait for all the dependencies for _, ch := range chs { <-ch @@ -206,13 +206,14 @@ func (g *AcyclicGraph) Walk(cb WalkFunc) error { defer errLock.Unlock() for _, dep := range deps { if errMap[dep] { + errMap[v] = true readyCh <- false return } } readyCh <- true - }(deps, depChs, readyCh) + }(v, deps, depChs, readyCh) // Start the goroutine that executes go func(v Vertex, doneCh chan<- struct{}, readyCh <-chan bool) { diff --git a/dag/dag_test.go b/dag/dag_test.go index e7b2db8d2264..93441775a946 100644 --- a/dag/dag_test.go +++ b/dag/dag_test.go @@ -226,8 +226,10 @@ func TestAcyclicGraphWalk_error(t *testing.T) { g.Add(1) g.Add(2) g.Add(3) + g.Add(4) + g.Connect(BasicEdge(4, 3)) g.Connect(BasicEdge(3, 2)) - g.Connect(BasicEdge(3, 1)) + g.Connect(BasicEdge(2, 1)) var visits []Vertex var lock sync.Mutex