-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
ws-manager: Fix consistency of disposed ws between ws-daemon and ws-manager #12028
Conversation
val, ok := m.finalizerMap.LoadAndDelete(workspaceID) | ||
if ok { | ||
cancelReq := val.(context.CancelFunc) | ||
cancelReq() | ||
} | ||
return false, &csapi.GitStatus{}, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move these lines upper?
gitpod/components/ws-manager/pkg/manager/monitor.go
Lines 1115 to 1126 in b8669f3
ctx, cancelReq := context.WithTimeout(ctx, time.Duration(m.manager.Config.Timeouts.ContentFinalization)) | |
m.finalizerMap.Store(workspaceID, cancelReq) | |
defer func() { | |
// we're done disposing - remove from the finalizerMap | |
val, ok := m.finalizerMap.LoadAndDelete(workspaceID) | |
if !ok { | |
return | |
} | |
cancelReq := val.(context.CancelFunc) | |
cancelReq() | |
}() |
Therefore, we can reuse the defer function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jenting Thanks for your feedback. fixed
b8669f3
to
7f7bf3e
Compare
/werft with-preview 👎 unknown command: with-preview |
started the job as gitpod-build-to-retry-prebuild.3 because the annotations in the pull request description changed |
Description
Due to a retry timing issue, there were cases where the workspace had already been successfully backed up on the ws-daemon side, but ws-manager was trying to back it up again with an error.
More detail
Related Issue(s)
Fixes #11710
How to test
Is there any good way to do this?
Release Notes
Documentation
Werft options: