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

Log the error from umount when it fails in the background #8561

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Log the error from umount when it fails in the background #8561

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
89b82f6
Log the error from umount when it fails in the background
vanja-p Mar 5, 2025
5dea406
fix build
vanja-p Mar 5, 2025
3995e18
remove duplicate call
vanja-p Mar 5, 2025
02dfe37
Merge branch 'master' into vanja-umount
vanja-p Mar 5, 2025
353e31f
add comment
vanja-p Mar 6, 2025
1436206
fix race and remove extra logging
vanja-p Mar 10, 2025
80976af
defer close
vanja-p Mar 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions enterprise/server/remote_execution/vbd/vbd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,21 @@ func (f *FS) Unmount(ctx context.Context) error {
// Unmount in the background to prevent tasks from being blocked if it
// hangs forever.
// Log an error if this happens, since this is a goroutine leak.
resultCh := make(chan error, 1)
resultCh := make(chan error)
go func() {
resultCh <- f.unmount(ctx)
defer close(resultCh)
err := f.unmount(ctx)
select {
case resultCh <- err:
// Since resultCh is unbuffered, this only happens when the outer
// function received from the channel and will return this err.
case <-ctx.Done():
if err != nil {
log.CtxErrorf(ctx, "Failed to unmount %s in the background after context was cancelled: %s", f.mountPath, err)
} else {
log.CtxInfof(ctx, "Unmounted %s in the background, even after the context was canceled", f.mountPath)
}
}
}()
select {
case err := <-resultCh:
Expand Down Expand Up @@ -159,7 +171,7 @@ func (f *FS) unmount(ctx context.Context) error {
// If we successfully unmounted, then the mount path should point to
// an empty dir. Remove it.
if err := os.Remove(f.mountPath); err != nil {
log.CtxErrorf(ctx, "Failed to unmount vbd: %s", err)
log.CtxErrorf(ctx, "Failed to remove vbd mount path %s: %s", f.mountPath, err)
}
}
if err := os.Remove(f.lockFile.Name()); err != nil {
Expand All @@ -168,11 +180,6 @@ func (f *FS) unmount(ctx context.Context) error {
if err := f.lockFile.Close(); err != nil {
log.CtxErrorf(ctx, "Failed to unlock vbd lock file: %s", err)
}
if ctx.Err() != nil {
log.CtxInfof(ctx, "Unmounted %s in the background, even after the context was canceled", f.mountPath)
} else {
log.CtxDebugf(ctx, "Unmounted %s", f.mountPath)
}
return err
}

Expand Down