From ee8cc627a0a6e0f01d1fbdb8e11e05d256cf6e1a Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 10 Jul 2020 10:16:45 -0400 Subject: [PATCH] don't store an entire Resource in each Instance The AbstractResourceInstance type was storing the entire Resource from the state, when it only needs the actual instance state. This would cause resources to consume memory on the order of n^2, where n in the number of instances of the resource. Rather than attaching the entire resource state, which includes copying each individual instance, only attach the ResourceInstance state, and extract out the provider address from the Resource. --- terraform/node_resource_abstract.go | 34 +++++++++++++++++------------ terraform/node_resource_destroy.go | 14 ++++-------- terraform/node_resource_refresh.go | 4 ++-- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index aada8d09c..051c7f444 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -101,11 +101,14 @@ type NodeAbstractResourceInstance struct { NodeAbstractResource Addr addrs.AbsResourceInstance - // The fields below will be automatically set using the Attach - // interfaces if you're running those transforms, but also be explicitly - // set if you already have that information. - ResourceState *states.Resource - Dependencies []addrs.ConfigResource + // These are set via the AttachState method. + instanceState *states.ResourceInstance + // storedProviderConfig is the provider address retrieved from the + // state, but since it is only stored in the whole Resource rather than the + // ResourceInstance, we extract it out here. + storedProviderConfig addrs.AbsProviderConfig + + Dependencies []addrs.ConfigResource } var ( @@ -287,11 +290,9 @@ func dottedInstanceAddr(tr addrs.ResourceInstance) string { // StateDependencies returns the dependencies saved in the state. func (n *NodeAbstractResourceInstance) StateDependencies() []addrs.ConfigResource { - if rs := n.ResourceState; rs != nil { - if s := rs.Instance(n.Addr.Resource.Key); s != nil { - if s.Current != nil { - return s.Current.Dependencies - } + if s := n.instanceState; s != nil { + if s.Current != nil { + return s.Current.Dependencies } } @@ -339,12 +340,12 @@ func (n *NodeAbstractResourceInstance) ProvidedBy() (addrs.ProviderConfig, bool) }, false } - // If we have state, then we will use the provider from there - if n.ResourceState != nil { + // See if we have a valid provider config from the state. + if n.storedProviderConfig.Provider.Type != "" { // An address from the state must match exactly, since we must ensure // we refresh/destroy a resource with the same provider configuration // that created it. - return n.ResourceState.ProviderConfig, true + return n.storedProviderConfig, true } // No provider configuration found; return a default address @@ -410,7 +411,12 @@ func (n *NodeAbstractResource) AttachResourceDependencies(deps []addrs.ConfigRes // GraphNodeAttachResourceState func (n *NodeAbstractResourceInstance) AttachResourceState(s *states.Resource) { - n.ResourceState = s + if s == nil { + log.Printf("[WARN] attaching nil state to %s", n.Addr) + return + } + n.instanceState = s.Instance(n.Addr.Resource.Key) + n.storedProviderConfig = s.ProviderConfig } // GraphNodeAttachResourceConfig diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 6fc24fee2..1df561b4a 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -63,11 +63,9 @@ func (n *NodeDestroyResourceInstance) CreateBeforeDestroy() bool { } // Otherwise check the state for a stored destroy order - if rs := n.ResourceState; rs != nil { - if s := rs.Instance(n.Addr.Resource.Key); s != nil { - if s.Current != nil { - return s.Current.CreateBeforeDestroy - } + if s := n.instanceState; s != nil { + if s.Current != nil { + return s.Current.CreateBeforeDestroy } } @@ -134,11 +132,7 @@ func (n *NodeDestroyResourceInstance) EvalTree() EvalNode { addr := n.ResourceInstanceAddr() // Get our state - rs := n.ResourceState - var is *states.ResourceInstance - if rs != nil { - is = rs.Instance(n.Addr.Resource.Key) - } + is := n.instanceState if is == nil { log.Printf("[WARN] NodeDestroyResourceInstance for %s with no state", addr) } diff --git a/terraform/node_resource_refresh.go b/terraform/node_resource_refresh.go index 8151ad3a0..8259470da 100644 --- a/terraform/node_resource_refresh.go +++ b/terraform/node_resource_refresh.go @@ -215,7 +215,7 @@ func (n *NodeRefreshableManagedResourceInstance) EvalTree() EvalNode { // Eval info is different depending on what kind of resource this is switch addr.Resource.Resource.Mode { case addrs.ManagedResourceMode: - if n.ResourceState == nil { + if n.instanceState == nil { log.Printf("[TRACE] NodeRefreshableManagedResourceInstance: %s has no existing state to refresh", addr) return n.evalTreeManagedResourceNoState() } @@ -253,7 +253,7 @@ func (n *NodeRefreshableManagedResourceInstance) evalTreeManagedResource() EvalN // This happened during initial development. All known cases were // fixed and tested but as a sanity check let's assert here. - if n.ResourceState == nil { + if n.instanceState == nil { err := fmt.Errorf( "No resource state attached for addr: %s\n\n"+ "This is a bug. Please report this to Terraform with your configuration\n"+