Skip to content

Commit

Permalink
Use request timeout for git service rpc (#20689)
Browse files Browse the repository at this point in the history
This enables git.Command's Run to optionally use the given context directly so its deadline will be respected. Otherwise, it falls back to the previous behavior of using the supplied timeout or a default timeout value of 360 seconds.

repo's serviceRPC() calls now use the context's deadline (which is unset/unlimited) instead of the default 6-minute timeout. This means that large repo clones will no longer arbitrarily time out on the upload-pack step, and pushes can take longer than 6 minutes on the receive-pack step.

Fixes #20680

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
  • Loading branch information
parnic and lunny authored Aug 6, 2022
1 parent 279e4e2 commit 2778990
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 13 deletions.
25 changes: 17 additions & 8 deletions modules/git/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,15 @@ func (c *Command) AddArguments(args ...string) *Command {
return c
}

// RunOpts represents parameters to run the command
// RunOpts represents parameters to run the command. If UseContextTimeout is specified, then Timeout is ignored.
type RunOpts struct {
Env []string
Timeout time.Duration
Dir string
Stdout, Stderr io.Writer
Stdin io.Reader
PipelineFunc func(context.Context, context.CancelFunc) error
Env []string
Timeout time.Duration
UseContextTimeout bool
Dir string
Stdout, Stderr io.Writer
Stdin io.Reader
PipelineFunc func(context.Context, context.CancelFunc) error
}

func commonBaseEnvs() []string {
Expand Down Expand Up @@ -171,7 +172,15 @@ func (c *Command) Run(opts *RunOpts) error {
desc = fmt.Sprintf("%s %s [repo_path: %s]", c.name, strings.Join(args, " "), opts.Dir)
}

ctx, cancel, finished := process.GetManager().AddContextTimeout(c.parentContext, opts.Timeout, desc)
var ctx context.Context
var cancel context.CancelFunc
var finished context.CancelFunc

if opts.UseContextTimeout {
ctx, cancel, finished = process.GetManager().AddContext(c.parentContext, desc)
} else {
ctx, cancel, finished = process.GetManager().AddContextTimeout(c.parentContext, opts.Timeout, desc)
}
defer finished()

cmd := exec.CommandContext(ctx, c.name, c.args...)
Expand Down
11 changes: 6 additions & 5 deletions routers/web/repo/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -474,11 +474,12 @@ func serviceRPC(ctx gocontext.Context, h serviceHandler, service string) {
cmd := git.NewCommand(h.r.Context(), service, "--stateless-rpc", h.dir)
cmd.SetDescription(fmt.Sprintf("%s %s %s [repo_path: %s]", git.GitExecutable, service, "--stateless-rpc", h.dir))
if err := cmd.Run(&git.RunOpts{
Dir: h.dir,
Env: append(os.Environ(), h.environ...),
Stdout: h.w,
Stdin: reqBody,
Stderr: &stderr,
Dir: h.dir,
Env: append(os.Environ(), h.environ...),
Stdout: h.w,
Stdin: reqBody,
Stderr: &stderr,
UseContextTimeout: true,
}); err != nil {
if err.Error() != "signal: killed" {
log.Error("Fail to serve RPC(%s) in %s: %v - %s", service, h.dir, err, stderr.String())
Expand Down

0 comments on commit 2778990

Please sign in to comment.