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

fix race condition in waiting for restic restores to complete #2201

Merged
merged 3 commits into from Jan 21, 2020

Conversation

@skriss
Copy link
Member

skriss commented Jan 15, 2020

Signed-off-by: Steve Kriss krisss@vmware.com

closes #2121

cc @dymurray

Basic validation on this looks good, but I'd like to do some more testing before merging so leaving in draft status.

@skriss skriss requested review from carlisia, nrb and ashish-amarnath Jan 15, 2020
@skriss skriss marked this pull request as ready for review Jan 15, 2020
@skriss

This comment has been minimized.

Copy link
Member Author

skriss commented Jan 15, 2020

looked good to me in testing, ready for review here.

skriss added 2 commits Jan 15, 2020
Signed-off-by: Steve Kriss <krisss@vmware.com>
changelog
Signed-off-by: Steve Kriss <krisss@vmware.com>
@skriss skriss force-pushed the skriss:fix-2121 branch from e4e2607 to 65e970f Jan 15, 2020
Copy link
Contributor

dymurray left a comment

Using a channel to wait for the restic restores to finish makes a lot of sense to me. Would be curious to see if this resolves the issue seen in our environment with the restic restore running out of disk space. Either way this change is an improvement to the way this currently functions.

pkg/restore/restore.go Outdated Show resolved Hide resolved
Copy link
Member

ashish-amarnath left a comment

Overall lgtm except for relocating the waiting log message into the go routine that does the waiting

Signed-off-by: Steve Kriss <krisss@vmware.com>
Copy link
Member

carlisia left a comment

lgtm 👍

@ashish-amarnath ashish-amarnath merged commit 421dcd4 into vmware-tanzu:master Jan 21, 2020
7 checks passed
7 checks passed
Run CI
Details
Header rules - velero No header rules processed
Details
Pages changed - velero 801 new files uploaded
Details
Redirect rules - velero No redirect rules processed
Details
DCO DCO
Details
Mixed content - velero No mixed content detected
Details
netlify/velero/deploy-preview Deploy preview ready!
Details
@skriss skriss deleted the skriss:fix-2121 branch Jan 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.