diff --git a/.changes/v1.13/ENHANCEMENTS-20250522-093102.yaml b/.changes/v1.13/ENHANCEMENTS-20250522-093102.yaml new file mode 100644 index 0000000000..5363103a43 --- /dev/null +++ b/.changes/v1.13/ENHANCEMENTS-20250522-093102.yaml @@ -0,0 +1,5 @@ +kind: ENHANCEMENTS +body: Performance fix for evaluating high cardinality resources +time: 2025-05-22T09:31:02.761383-04:00 +custom: + Issue: "26355" diff --git a/internal/logging/logging.go b/internal/logging/logging.go index b15f974330..5a479c01a9 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -27,6 +27,10 @@ const ( envLogProvider = "TF_LOG_PROVIDER" envLogCloud = "TF_LOG_CLOUD" envLogStacks = "TF_LOG_STACKS" + + // This variable is defined here for consistency, but it is used by the + // graph builder directly to change how much output to generate. + envGraphTrace = "TF_GRAPH_TRACE" ) var ( @@ -44,6 +48,8 @@ var ( panics: make(map[string][]string), maxLines: 100, } + + GraphTrace = os.Getenv(envGraphTrace) != "" ) func init() { diff --git a/internal/moduletest/graph/apply.go b/internal/moduletest/graph/apply.go index 632545bcbf..7d58595dd9 100644 --- a/internal/moduletest/graph/apply.go +++ b/internal/moduletest/graph/apply.go @@ -72,8 +72,6 @@ func (n *NodeTestRun) testApply(ctx *EvalContext, variables terraform.InputValue return } - n.AddVariablesToConfig(variables) - if ctx.Verbose() { schemas, diags := tfCtx.Schemas(config, updated) diff --git a/internal/moduletest/graph/plan.go b/internal/moduletest/graph/plan.go index f803d84999..f59dc56744 100644 --- a/internal/moduletest/graph/plan.go +++ b/internal/moduletest/graph/plan.go @@ -40,8 +40,6 @@ func (n *NodeTestRun) testPlan(ctx *EvalContext, variables terraform.InputValues return } - n.AddVariablesToConfig(variables) - if ctx.Verbose() { schemas, diags := tfCtx.Schemas(config, plan.PriorState) diff --git a/internal/moduletest/graph/variables.go b/internal/moduletest/graph/variables.go index 0c19524683..e48d5ca41c 100644 --- a/internal/moduletest/graph/variables.go +++ b/internal/moduletest/graph/variables.go @@ -7,13 +7,13 @@ import ( "fmt" "github.com/hashicorp/hcl/v2" + "github.com/zclconf/go-cty/cty" + "github.com/hashicorp/terraform/internal/addrs" - "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/lang/langrefs" hcltest "github.com/hashicorp/terraform/internal/moduletest/hcl" "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" ) // GetVariables builds the terraform.InputValues required for the provided run @@ -203,37 +203,3 @@ func (n *NodeTestRun) FilterVariablesToModule(values terraform.InputValues) (mod } return moduleVars, testOnlyVars, diags } - -// AddVariablesToConfig extends the provided config to ensure it has definitions -// for all specified variables. -// -// This function is essentially the opposite of FilterVariablesToConfig which -// makes the variables match the config rather than the config match the -// variables. -func (n *NodeTestRun) AddVariablesToConfig(variables terraform.InputValues) { - run := n.run - // If we have got variable values from the test file we need to make sure - // they have an equivalent entry in the configuration. We're going to do - // that dynamically here. - - // First, take a backup of the existing configuration so we can easily - // restore it later. - currentVars := make(map[string]*configs.Variable) - for name, variable := range run.ModuleConfig.Module.Variables { - currentVars[name] = variable - } - - for name, value := range variables { - if _, exists := run.ModuleConfig.Module.Variables[name]; exists { - continue - } - - run.ModuleConfig.Module.Variables[name] = &configs.Variable{ - Name: name, - Type: value.Value.Type(), - ConstraintType: value.Value.Type(), - DeclRange: value.SourceRange.ToHCL(), - } - } - -} diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go index 095830db79..e3151ea654 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -658,6 +658,16 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc instances[key] = value } + // Proactively read out all the resource changes before iteration. Not only + // does GetResourceInstanceChange have to iterate over all instances + // internally causing an n^2 lookup, but Changes is also a major point of + // lock contention. + resChanges := d.Evaluator.Changes.GetChangesForConfigResource(addr.InModule(moduleConfig.Path)) + instChanges := addrs.MakeMap[addrs.AbsResourceInstance, *plans.ResourceInstanceChange]() + for _, ch := range resChanges { + instChanges.Put(ch.Addr, ch) + } + // Decode all instances in the current state pendingDestroy := d.Operation == walkDestroy for key, is := range rs.Instances { @@ -675,7 +685,7 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc } instAddr := addr.Instance(key).Absolute(d.ModulePath) - change := d.Evaluator.Changes.GetResourceInstanceChange(instAddr, addrs.NotDeposed) + change := instChanges.Get(instAddr) if change != nil { // Don't take any resources that are yet to be deleted into account. // If the referenced resource is CreateBeforeDestroy, then orphaned diff --git a/internal/terraform/graph_builder.go b/internal/terraform/graph_builder.go index 4b7d6f781b..d496ca9d49 100644 --- a/internal/terraform/graph_builder.go +++ b/internal/terraform/graph_builder.go @@ -45,11 +45,14 @@ func (b *BasicGraphBuilder) Build(path addrs.ModuleInstance) (*Graph, tfdiags.Di log.Printf("[TRACE] Executing graph transform %T", step) err := step.Transform(g) - if thisStepStr := g.StringWithNodeTypes(); thisStepStr != lastStepStr { - log.Printf("[TRACE] Completed graph transform %T with new graph:\n%s ------", step, logging.Indent(thisStepStr)) - lastStepStr = thisStepStr - } else { - log.Printf("[TRACE] Completed graph transform %T (no changes)", step) + + if logging.GraphTrace { + if thisStepStr := g.StringWithNodeTypes(); thisStepStr != lastStepStr { + log.Printf("[TRACE] Completed graph transform %T with new graph:\n%s ------", step, logging.Indent(thisStepStr)) + lastStepStr = thisStepStr + } else { + log.Printf("[TRACE] Completed graph transform %T (no changes)", step) + } } if err != nil { @@ -65,6 +68,11 @@ func (b *BasicGraphBuilder) Build(path addrs.ModuleInstance) (*Graph, tfdiags.Di } } + if !logging.GraphTrace { + // print the graph at least once for normal trace logs + log.Printf("[TRACE] Completed graph transform:\n%s ------", g.StringWithNodeTypes()) + } + // Return early if the graph validation is skipped // This behavior is currently only used by the graph command // which only wants to display the dot representation of the graph