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

Cancel root module operations on shutdown #195

Closed
radeksimko opened this issue Jun 30, 2020 · 1 comment · Fixed by #219
Closed

Cancel root module operations on shutdown #195

radeksimko opened this issue Jun 30, 2020 · 1 comment · Fixed by #219
Assignees
Labels
bug Something isn't working
Milestone

Comments

@radeksimko
Copy link
Member

radeksimko commented Jun 30, 2020

Server Version

0.4.0

Expected Behavior

On shutdown of the server (e.g. initiated by quitting the IDE) it should cancel any operations running on the root module, such as:

  • terraform version
  • obtaining schema via terraform providers schema -json
  • parsing module manifest

Actual Behavior

Terraform executions will get cancelled either way, because they either inherit the session context or watcher context, both of which get cancelled as part of the shutdown:

svc.logger.Println("Stopping watcher for session ...")
err := svc.ww.Stop()
if err != nil {
svc.logger.Println("Unable to stop watcher for session:", err)
}
svc.stopSession()

A new challenge arises once we make walking asynchronous.
While the walker itself will be cancellable, the operations launched by the walker such as adding new root modules (and then performing any of the above) will retain context of the session and therefore can't be cancelled as part of walker cancellation.

This may result in multiple seconds of delay on shutdown while waiting for terraform to finish executing.

Further Context

Each root module gets its own non-cancellable context at the moment:

rm, err := rmm.newRootModule(context.Background(), dir)

Generally the context-related logic in rootmodule needs some overhaul to avoid persisting context.

This may require some more explicit workflow management, explicit cancellation of each individual RootModule, and perhaps explicit cancellation of RootModuleManager (which will in turn cancel and remove any tracked root modules).

In other words we should only be persisting context.CancelFunc for each root module, but not the full context.

@radeksimko radeksimko added the bug Something isn't working label Jun 30, 2020
@radeksimko radeksimko added this to the v0.4.2 milestone Jul 3, 2020
@radeksimko radeksimko self-assigned this Jul 7, 2020
@radeksimko radeksimko modified the milestones: v0.4.2, v0.5.0 Jul 10, 2020
@ghost
Copy link

ghost commented Aug 8, 2020

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

@ghost ghost locked and limited conversation to collaborators Aug 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant