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.

Sign up for GitHub

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

Jump to bottom

Remove some destroy cycles #22976

Merged
merged 4 commits into from Oct 24, 2019
Merged

Remove some destroy cycles #22976

merged 4 commits into from Oct 24, 2019

Commits on Oct 2, 2019

  1. failing test for module instance replace cycle 

    Destroy-time references are not correctly or fully inverted when
crossing module boundaries, causing cycle during apply.
    @jbardin
    jbardin committed Oct 2, 2019
    Copy the full SHA
    94cd268 View commit details
    Browse the repository at this point in the history

  2. do not connect destroy and resource nodes 

    Destroy nodes do not need to be connected to the resource (prepare
state) node when adding them to the graph. Destroy nodes already have a
complete state in the graph (which is being destroyed), any references
will be added in the ReferenceTransformer, and the proper
connection to the create node will be added in the
DestroyEdgeTransformer.

Under normal circumstances this makes no difference, as create and
destroy nodes always have an dependency, so having the prepare state
handled before both only linearizes the operation slightly in the
normal destroy-then-create scenario.

However if there is a dependency on a resource being replaced in another
module, there will be a dependency between the destroy nodes in each
module (to complete the destroy ordering), while the resource node will
depend on the variable->output->resource chain. If both the destroy and
create nodes depend on the resource node, there will be a cycle.
    @jbardin
    jbardin committed Oct 2, 2019
    Copy the full SHA
    3d33974 View commit details
    Browse the repository at this point in the history

  3. failing test with destroy cycle 

    after the previous commit the cycle is broken, but we can't evaluate
resource counts that depends on data sources being destroyed.
    @jbardin
    jbardin committed Oct 2, 2019
    Copy the full SHA
    407e84e View commit details
    Browse the repository at this point in the history

  4. prune unused resources from apply 

    If a resource is only destroying instances, there is no reason to
prepare the state and we can remove the Resource (prepare state) nodes.
They normally have pose no issue, but if the instances are being
destroyed along with their dependencies, the resource node may fail to
evaluate due to the missing dependencies (since destroy happens in the
reverse order).

These failures were previously blocked by there being a cycle when the
destroy nodes were directly attached to the resource nodes.
    @jbardin
    jbardin committed Oct 2, 2019
    Copy the full SHA
    4ece418 View commit details
    Browse the repository at this point in the history