Skip to content

Commit

Permalink
Backport of optimize and disable resource graph (#35074)
Browse files Browse the repository at this point in the history
* optimize reducePreservingRelationships

The reducePreservingRelationships method is strictly removing nodes and
passing through the connecting edges. This does not take multiple
rounds if the removal is done immediately, so we can make the process
much simpler.

* don't create the deferrals graph if not needed

When deferrals are not being used, don't spend time building the
deferrals graph.

---------

Co-authored-by: James Bardin <j.bardin@gmail.com>
  • Loading branch information
liamcervante and jbardin authored Apr 24, 2024
1 parent 6091b3a commit b8d4c3e
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 44 deletions.
6 changes: 6 additions & 0 deletions internal/terraform/context_plan2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4818,6 +4818,12 @@ func TestContext2Apply_externalDependencyDeferred(t *testing.T) {

cfg := testModuleInline(t, map[string]string{
"main.tf": `
// TEMP: unknown for_each currently requires an experiment opt-in.
// We should remove this block if the experiment gets stabilized.
terraform {
experiments = [unknown_instances]
}
resource "test" "a" {
name = "a"
}
Expand Down
25 changes: 19 additions & 6 deletions internal/terraform/context_walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/checks"
"github.com/hashicorp/terraform/internal/configs"
"github.com/hashicorp/terraform/internal/experiments"
"github.com/hashicorp/terraform/internal/instances"
"github.com/hashicorp/terraform/internal/moduletest/mocking"
"github.com/hashicorp/terraform/internal/namedvals"
Expand Down Expand Up @@ -165,12 +166,24 @@ func (c *Context) graphWalker(graph *Graph, operation walkOperation, opts *graph
}
}

// We'll produce a derived graph that only includes the static resource
// blocks, since we need that for deferral tracking.
resourceGraph := graph.ResourceGraph()
deferred := deferring.NewDeferred(resourceGraph)
if opts.ExternalDependencyDeferred {
deferred.SetExternalDependencyDeferred()
deferralsAllowed := false
opts.Config.DeepEach(func(c *configs.Config) {
if c.Module != nil && c.Module.ActiveExperiments.Has(experiments.UnknownInstances) {
deferralsAllowed = true
}
})

var deferred *deferring.Deferred
if deferralsAllowed {
// We'll produce a derived graph that only includes the static resource
// blocks, since we need that for deferral tracking.
resourceGraph := graph.ResourceGraph()
deferred = deferring.NewDeferred(resourceGraph)
if opts.ExternalDependencyDeferred {
deferred.SetExternalDependencyDeferred()
}
} else {
deferred = deferring.NewDeferred(addrs.NewDirectedGraph[addrs.ConfigResource]())
}

return &ContextGraphWalker{
Expand Down
66 changes: 28 additions & 38 deletions internal/terraform/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,24 +191,35 @@ func (g *Graph) ResourceGraph() addrs.DirectedGraph[addrs.ConfigResource] {
// and then using that temporary graph to construct the final graph to
// return.

log.Printf("[TRACE] ResourceGraph: copying source graph\n")
tmpG := Graph{}
tmpG.Subsume(&g.Graph)
log.Printf("[TRACE] ResourceGraph: reducing graph\n")
tmpG.reducePreservingRelationships(func(n dag.Vertex) bool {
_, ret := n.(GraphNodeConfigResource)
return ret
})
log.Printf("[TRACE] ResourceGraph: TransitiveReduction\n")

// The resulting graph could have many more edges now, but alternate paths
// are not a problem for the deferral system, so we may choose not to run
// this as it may be very time consuming. The reducePreservingRelationships
// method also doesn't add many (if any) redundant new edges to most graphs.
tmpG.TransitiveReduction()

log.Printf("[TRACE] ResourceGraph: creating address graph\n")
ret := addrs.NewDirectedGraph[addrs.ConfigResource]()
for _, n := range tmpG.Vertices() {
sourceR := n.(GraphNodeConfigResource)
sourceAddr := sourceR.ResourceAddr()
ret.Add(sourceAddr)
for _, dn := range tmpG.DownEdges(n) {
targetR := dn.(GraphNodeConfigResource)

ret.AddDependency(sourceAddr, targetR.ResourceAddr())
}
}
log.Printf("[TRACE] ResourceGraph: completed with %d nodes\n", len(ret.AllNodes()))
return ret
}

Expand All @@ -217,46 +228,25 @@ func (g *Graph) ResourceGraph() addrs.DirectedGraph[addrs.ConfigResource] {
// edges to preserve the dependency relationships for all of the nodes
// that still remain.
func (g *Graph) reducePreservingRelationships(keepNode func(dag.Vertex) bool) {
// This is a naive algorithm for now. Maybe we'll improve it later.

// We'll keep iterating as long as we find new edges to add because we
// might need to bridge across multiple nodes that we're going to remove
// in order to retain the relationships, and one iteration can only
// bridge a single node at a time.
changed := true
for changed {
changed = false
for _, n := range g.Vertices() {
if keepNode(n) {
continue
}

// If we're not going to keep this node then we need to connect
// all of its dependents to all of its dependencies so that the
// ordering is still preserved for those nodes that remain.
// However, this will often generate more edges than are strictly
// required and so it could be productive to run a transitive
// reduction afterwards.
dependents := g.UpEdges(n)
dependencies := g.DownEdges(n)
for dependent := range dependents {
for dependency := range dependencies {
edge := dag.BasicEdge(dependent, dependency)
if !g.HasEdge(edge) {
g.Connect(edge)
changed = true
}
}
}
for _, n := range g.Vertices() {
if keepNode(n) {
continue
}
}

// With all of the extra supporting edges in place, we can now safely
// remove the nodes we aren't going to keep without changing the
// relationships of the remaining nodes.
for _, n := range g.Vertices() {
if !keepNode(n) {
g.Remove(n)
// If we're not going to keep this node then we need to connect
// all of its dependents to all of its dependencies so that the
// ordering is still preserved for those nodes that remain.
// However, this will often generate more edges than are strictly
// required and so it could be productive to run a transitive
// reduction afterwards.
dependents := g.UpEdges(n)
dependencies := g.DownEdges(n)
for dependent := range dependents {
for dependency := range dependencies {
edge := dag.BasicEdge(dependent, dependency)
g.Connect(edge)
}
}
g.Remove(n)
}
}

0 comments on commit b8d4c3e

Please sign in to comment.