Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of Apply optimizations for handling of condition checks into v1.3 #32134

Merged

Conversation

teamterraform
Copy link
Contributor

Backport

This PR is auto-generated from #32123 to be assessed for backporting due to the inclusion of the label 1.3-backport.

The below text is copied from the body of the original PR.


The handling of conditions during apply can have adverse affects on very large configs, due to the handling of all existing instances even when there are no changes. With extremely large configurations, the execution time can go from a few minutes to many hours only for minor changes.

The first step is to ensure empty CheckResults in the state are normalized to prevent the state from being re-written when there are no semantic differences. Empty and nil slices are not handled consistently overall, but we will use an empty slice in the state zero value to match the other top-level structures.

Next we can skip adding NoOp changes to the apply graph when there are no conditions at all to evaluate from the configuration. The nodes themselves can have significant processing overhead when assembling the graph, especially for steps like recalculating individual instance dependencies.

Finally we need to avoid re-planning each instance and re-writing the state for every NoOp change during apply. The whole process has gotten more convoluted with the addition of condition checks, as preconditions are done during the NodeAbstractResourceInstance.plan method, but postconditions are handled outside of the abstract instance in the NodeApplyableResourceInstance.managedResourceExecute method, and repetition data needs to be shared between all these. I pulled the repetition data out of apply to simplify the early return in the short-term, but there is definitely some more refactoring to be done in this area.

Fixes #32060
Fixes #32071

@teamterraform teamterraform force-pushed the backport/jbardin/noop-apply-optimizations/loudly-choice-squirrel branch from 6a7d3a1 to eba89ba Compare November 1, 2022 20:33
@jbardin jbardin merged commit ba8c9c8 into v1.3 Nov 1, 2022
@jbardin jbardin deleted the backport/jbardin/noop-apply-optimizations/loudly-choice-squirrel branch November 1, 2022 20:51
@github-actions
Copy link

github-actions bot commented Nov 1, 2022

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

@github-actions
Copy link

github-actions bot commented Dec 2, 2022

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants