Skip to content

Commit

Permalink
Improve the DAG building and link detection
Browse files Browse the repository at this point in the history
Only consider link B -> A once. If A depends from B in N different
ways, consider that a single link in the dag. Considering them
different won't help us detect cycles nor scheduling tasks.

In the cycle detection logic, use a string builder instead of '+'
to concatenate strings, since it more efficient.
  • Loading branch information
afrittoli authored and tekton-robot committed Nov 17, 2020
1 parent fda7a81 commit 719e304
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 3 deletions.
12 changes: 11 additions & 1 deletion pkg/apis/pipeline/v1beta1/pipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package v1beta1
import (
"github.com/tektoncd/pipeline/pkg/reconciler/pipeline/dag"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
)

// +genclient
Expand Down Expand Up @@ -174,7 +175,16 @@ func (pt PipelineTask) Deps() []string {
deps = append(deps, pt.resourceDeps()...)
deps = append(deps, pt.orderingDeps()...)

return deps
uniqueDeps := sets.NewString()
for _, w := range deps {
if uniqueDeps.Has(w) {
continue
}
uniqueDeps.Insert(w)

}

return uniqueDeps.List()
}

func (pt PipelineTask) resourceDeps() []string {
Expand Down
6 changes: 5 additions & 1 deletion pkg/reconciler/pipeline/dag/dag.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,16 @@ func linkPipelineTasks(prev *Node, next *Node) error {
}

func visit(currentName string, nodes []*Node, path []string, visited map[string]bool) error {
var sb strings.Builder
for _, n := range nodes {
path = append(path, n.Task.HashKey())
if _, ok := visited[n.Task.HashKey()]; ok {
return errors.New(getVisitedPath(path))
}
visited[currentName+"."+n.Task.HashKey()] = true
sb.WriteString(currentName)
sb.WriteByte('.')
sb.WriteString(n.Task.HashKey())
visited[sb.String()] = true
if err := visit(n.Task.HashKey(), n.Prev, path, visited); err != nil {
return err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ func buildPipelineStateWithLargeDepencyGraph(t *testing.T) PipelineRunState {
TaskSpec: &task.Spec,
},
}}
for i := 2; i < 60; i++ {
for i := 2; i < 400; i++ {
dependFrom := 1
if i > 10 {
if i%10 == 0 {
Expand Down

0 comments on commit 719e304

Please sign in to comment.