hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
41.49k stars 9.38k forks source link

remove extra state.DeepCopy from updateStateHook #35164

Closed jbardin closed 4 weeks ago

jbardin commented 1 month ago

All state implementations copy the given state when storing it to temporary or permanent storage^1^3, either explicitly with DeepCopy, or implicitly by serializing the state. Since updateStateHook is called with every resource instance state change, a second call to DeepCopy with that hook doubles the overhead of PostStateUpdate which may be a significant portion of the running time.

Since all Hook implementations which handle PostStateUpdate (which is exactly 1 in non-test code^5) only call state methods that eventually copy the state, we can remove the extra copy from the updateStateHook itself. The state was already locked for the duration of the PostStateUpdate call previously, so no additional delay should result.

For consistency this also updates the documentation for PostStateUpdate to record the new caller and callee expectations based on the existing implementation. The Hook interface predated the statemgr which provides a better interface for synchronizing state access and therefor required extra copying at that time, while now we can more easily declare that the method requires concurrent access be locked for the duration of the call.

Fixes #35113

Target Release

v1.9

alexott commented 1 month ago

That’s great! Thank you very much! We initially looked into updateStateHook, but weren’t sure that all implementations will have their own deep copy…

github-actions[bot] commented 4 weeks ago

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.