Skip to content

[ws-manager] fix calling dispose multiple times #9803

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

Merged
merged 1 commit into from
May 6, 2022
Merged

Conversation

sagor999
Copy link
Contributor

@sagor999 sagor999 commented May 5, 2022

Description

Correct fix for sometimes calling dispose multiple times, which can result in incorrect error about backup failed error.

Related Issue(s)

Part of #9475
Fixes #9598
Fixes #8326

How to test

Start and stop workspaces. They should always shutdown correctly now, previously sometimes they would error out with failed backup error due to race condition.

Release Notes

[ws-manager] fix sometimes workspaces fail with backup not found error

Documentation

@sagor999 sagor999 requested a review from a team May 5, 2022 21:18
@github-actions github-actions bot added the team: workspace Issue belongs to the Workspace team label May 5, 2022
@roboquat roboquat merged commit cc893be into main May 6, 2022
@roboquat roboquat deleted the pavel/dispose-fix branch May 6, 2022 09:01
@csweichel
Copy link
Contributor

@sagor999 why did this even happen? The finalizer map should have prevented that in the first place. Does ws-manager restart?

Also, this really needs tests. There's an actOnPodEvent test which should capture this behaviour in a fixture. I'm somewhat surprised that there are not fixture changes.

@sagor999
Copy link
Contributor Author

sagor999 commented May 9, 2022

@csweichel I think it is a race issue.
Finalizer map is cleaned up when doFinalize has finished, if after that something triggers actOnPodEvent, it will attempt to run finalizer again. Hence adding annotation to make sure nothing will trigger it attempting to run finalizer.

Let me know if there is a better way to fix this.

@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels May 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note size/S team: workspace Issue belongs to the Workspace team
Projects
None yet
4 participants