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

Jump to bottom

Tweak ETXTBSY retry, and be helpful for ENOENT #2736

Merged
merged 2 commits into from
Apr 22, 2024
Merged

Tweak ETXTBSY retry, and be helpful for ENOENT #2736

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
41b5bf0
Tweak ETXTBSY retry on hook wrapper
DrJosh9000 Apr 22, 2024
6420d0b
Be helpful if the error is ENOENT
DrJosh9000 Apr 22, 2024
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
19 changes: 12 additions & 7 deletions internal/job/executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"slices"
"strconv"
"strings"
"syscall"
"time"

"github.com/buildkite/agent/v3/agent/plugin"
Expand Down Expand Up @@ -449,16 +450,20 @@ func (e *Executor) runWrappedShellScriptHook(ctx context.Context, hookName strin
e.shell.Promptf("%s", process.FormatCommand(cleanHookPath, []string{}))
}

const maxHookRetry = 3
const maxHookRetry = 30

// Run the wrapper script
if err := roko.NewRetrier(
roko.WithStrategy(roko.Constant(time.Second)),
roko.WithStrategy(roko.Constant(100*time.Millisecond)),
roko.WithMaxAttempts(maxHookRetry),
).DoWithContext(ctx, func(r *roko.Retrier) error {
// Run the script and only retry on fork/exec errors
// Run the script and only retry on ETXTBSY.
// This error occurs because of an unavoidable race between forking
// (which acquires open file descriptors of the parent process) and
// writing an executable (the script wrapper).
// See https://github.com/golang/go/issues/22315.
err := e.shell.RunScript(ctx, script.Path(), hookCfg.Env)
if perr := new(os.PathError); errors.As(err, &perr) && perr.Op == "fork/exec" {
if errors.Is(err, syscall.ETXTBSY) {
return err
}
r.Break()
Expand All @@ -476,9 +481,9 @@ func (e *Executor) runWrappedShellScriptHook(ctx context.Context, hookName strin
}
}

// If the error is from fork/exec, then inspect the file to see why it failed to be executed,
// even after the retry
if perr := new(os.PathError); errors.As(err, &perr) && perr.Op == "fork/exec" {
if perr := new(os.PathError); errors.As(err, &perr) && errors.Is(perr.Err, syscall.ETXTBSY) {
// If the error is ETXTBSY, then inspect the file to see what
// process had it open for write and log something helpful
logOpenedHookInfo(e.shell.Logger, e.Debug, hookName, perr.Path)
}

Expand Down