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

Make Requests Processes and create process hierarchy. Associate OpenRepository with context. #17125

Merged
merged 46 commits into from
Nov 30, 2021
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
3a7e682
Move process to create contexts
zeripath Sep 18, 2021
47bda44
display children processes
zeripath Sep 18, 2021
cd16cbb
Make requests a process
zeripath Sep 18, 2021
12039b9
Add context to repo and add ctx to OpenRepository
zeripath Sep 18, 2021
d2b01e4
minor comments
zeripath Sep 22, 2021
659fcf6
Merge remote-tracking branch 'origin/main' into request-as-process
zeripath Sep 23, 2021
9598eca
fix lint and children lock
zeripath Sep 23, 2021
3344e26
Merge remote-tracking branch 'origin/main' into request-as-process
zeripath Oct 10, 2021
7cf7749
separate remove and cancel functions
zeripath Oct 10, 2021
a8e228e
associate repo functions with the repo context
zeripath Oct 10, 2021
08b77d1
fix lint
zeripath Oct 13, 2021
47b0614
Merge remote-tracking branch 'origin/main' into request-as-process
zeripath Oct 13, 2021
518b79e
Merge remote-tracking branch 'origin/main' into request-as-process
zeripath Oct 15, 2021
9bb820b
Simplify PID to strings using the time of start plus/minus a counter
zeripath Oct 15, 2021
9895680
extract process out of manager.go
zeripath Oct 15, 2021
afc5b41
fix test
zeripath Oct 15, 2021
7446c87
Merge remote-tracking branch 'origin/main' into request-as-process
zeripath Oct 15, 2021
86ab980
Merge branch 'main' into request-as-process
zeripath Oct 17, 2021
4ce4614
Make the Mirror Queue a queue (#17326)
zeripath Oct 17, 2021
633b041
make mirroring a process
zeripath Oct 17, 2021
377a384
Ensure that mirrors are al within the same context
zeripath Oct 17, 2021
32c58ee
Merge remote-tracking branch 'origin/main' into request-as-process
zeripath Oct 17, 2021
71dec99
add clarity to the difference between cancel and remove
zeripath Oct 19, 2021
9e95fdb
add explanatory notes for remove and close
zeripath Oct 19, 2021
3010f59
Merge remote-tracking branch 'origin/main' into request-as-process
zeripath Oct 19, 2021
9cc97d0
explicitly name the arguments in the blame reader
zeripath Oct 19, 2021
e4aebfb
Change remove to finished
zeripath Oct 20, 2021
217fbf7
update blame documentation
zeripath Oct 20, 2021
6b6ac80
as per review
zeripath Oct 20, 2021
0fcbc38
Merge remote-tracking branch 'origin/main' into request-as-process
zeripath Oct 27, 2021
e06216b
Close the cat-file batch and checks after the context cancellation
zeripath Oct 27, 2021
2062e43
Merge remote-tracking branch 'origin/main' into request-as-process
zeripath Oct 28, 2021
46c2b7a
Merge branch 'main' into request-as-process
zeripath Nov 1, 2021
37bfa14
Merge branch 'main' into request-as-process
zeripath Nov 5, 2021
ad2e278
Merge remote-tracking branch 'origin/main' into request-as-process
zeripath Nov 16, 2021
6062b04
Merge remote-tracking branch 'origin/main' into request-as-process
zeripath Nov 20, 2021
59dc919
Merge remote-tracking branch 'origin/main' into request-as-process
zeripath Nov 26, 2021
347e6a8
Merge branch 'main' into request-as-process
lafriks Nov 28, 2021
fd86412
Merge branch 'main' into request-as-process
lunny Nov 28, 2021
0d1ae72
Merge remote-tracking branch 'origin/main' into request-as-process
zeripath Nov 28, 2021
1d565bb
Merge branch 'main' into request-as-process
zeripath Nov 29, 2021
bbe69c8
Merge branch 'main' into request-as-process
zeripath Nov 30, 2021
1203fa9
Ensure that http requests use the same context as the request
zeripath Nov 30, 2021
772d31d
use the repo context in the diff
zeripath Nov 30, 2021
37f0716
improve code documentation
zeripath Nov 30, 2021
ec6b663
use the gitrepo context
zeripath Nov 30, 2021
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
2 changes: 1 addition & 1 deletion cmd/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ func runHookPostReceive(c *cli.Context) error {
defer cancel()

// First of all run update-server-info no matter what
if _, err := git.NewCommand("update-server-info").SetParentContext(ctx).Run(); err != nil {
if _, err := git.NewCommandContext(ctx, "update-server-info").Run(); err != nil {
return fmt.Errorf("Failed to call 'git update-server-info': %v", err)
}

Expand Down
4 changes: 2 additions & 2 deletions modules/context/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) {
return
}

gitRepo, err := git.OpenRepository(models.RepoPath(userName, repoName))
gitRepo, err := git.OpenRepositoryCtx(ctx, models.RepoPath(userName, repoName))
if err != nil {
if strings.Contains(err.Error(), "repository does not exist") || strings.Contains(err.Error(), "no such file or directory") {
log.Error("Repository %-v has a broken repository on the file system: %s Error: %v", ctx.Repo.Repository, ctx.Repo.Repository.RepoPath(), err)
Expand Down Expand Up @@ -792,7 +792,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context

if ctx.Repo.GitRepo == nil {
repoPath := models.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
ctx.Repo.GitRepo, err = git.OpenRepository(repoPath)
ctx.Repo.GitRepo, err = git.OpenRepositoryCtx(ctx, repoPath)
if err != nil {
ctx.ServerError("RepoRef Invalid repo "+repoPath, err)
return
Expand Down
18 changes: 7 additions & 11 deletions modules/git/batch_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,15 @@ type WriteCloserError interface {
}

// CatFileBatchCheck opens git cat-file --batch-check in the provided repo and returns a stdin pipe, a stdout reader and cancel function
func CatFileBatchCheck(repoPath string) (WriteCloserError, *bufio.Reader, func()) {
func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError, *bufio.Reader, func()) {
batchStdinReader, batchStdinWriter := io.Pipe()
batchStdoutReader, batchStdoutWriter := io.Pipe()
ctx, ctxCancel := context.WithCancel(DefaultContext)
ctx, ctxCancel := context.WithCancel(ctx)
closed := make(chan struct{})
cancel := func() {
_ = batchStdinReader.Close()
_ = batchStdinWriter.Close()
_ = batchStdoutReader.Close()
_ = batchStdoutWriter.Close()
ctxCancel()
_ = batchStdoutReader.Close()
_ = batchStdinWriter.Close()
<-closed
}

Expand Down Expand Up @@ -67,19 +65,17 @@ func CatFileBatchCheck(repoPath string) (WriteCloserError, *bufio.Reader, func()
}

// CatFileBatch opens git cat-file --batch in the provided repo and returns a stdin pipe, a stdout reader and cancel function
func CatFileBatch(repoPath string) (WriteCloserError, *bufio.Reader, func()) {
func CatFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufio.Reader, func()) {
// We often want to feed the commits in order into cat-file --batch, followed by their trees and sub trees as necessary.
// so let's create a batch stdin and stdout
batchStdinReader, batchStdinWriter := io.Pipe()
batchStdoutReader, batchStdoutWriter := nio.Pipe(buffer.New(32 * 1024))
ctx, ctxCancel := context.WithCancel(DefaultContext)
ctx, ctxCancel := context.WithCancel(ctx)
closed := make(chan struct{})
cancel := func() {
_ = batchStdinReader.Close()
ctxCancel()
_ = batchStdinWriter.Close()
_ = batchStdoutReader.Close()
_ = batchStdoutWriter.Close()
ctxCancel()
<-closed
}

Expand Down
39 changes: 19 additions & 20 deletions modules/git/blame.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ type BlamePart struct {

// BlameReader returns part of file blame one by one
type BlameReader struct {
cmd *exec.Cmd
pid int64
output io.ReadCloser
reader *bufio.Reader
lastSha *string
cancel context.CancelFunc
cmd *exec.Cmd
output io.ReadCloser
reader *bufio.Reader
lastSha *string
cancel context.CancelFunc // Cancels the context that this reader runs in
finished process.FinishedFunc // Tells the process manager we're finished and it can remove the associated process from the process table
}

var shaLineRegex = regexp.MustCompile("^([a-z0-9]{40})")
Expand Down Expand Up @@ -100,8 +100,8 @@ func (r *BlameReader) NextPart() (*BlamePart, error) {

// Close BlameReader - don't run NextPart after invoking that
func (r *BlameReader) Close() error {
defer process.GetManager().Remove(r.pid)
r.cancel()
defer r.finished() // Only remove the process from the process table when the underlying command is closed
r.cancel() // However, first cancel our own context early
wxiaoguang marked this conversation as resolved.
Show resolved Hide resolved

_ = r.output.Close()

Expand All @@ -114,7 +114,7 @@ func (r *BlameReader) Close() error {

// CreateBlameReader creates reader for given repository, commit and file
func CreateBlameReader(ctx context.Context, repoPath, commitID, file string) (*BlameReader, error) {
gitRepo, err := OpenRepository(repoPath)
gitRepo, err := OpenRepositoryCtx(ctx, repoPath)
if err != nil {
return nil, err
}
Expand All @@ -125,32 +125,31 @@ func CreateBlameReader(ctx context.Context, repoPath, commitID, file string) (*B

func createBlameReader(ctx context.Context, dir string, command ...string) (*BlameReader, error) {
// Here we use the provided context - this should be tied to the request performing the blame so that it does not hang around.
ctx, cancel := context.WithCancel(ctx)
ctx, cancel, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("GetBlame [repo_path: %s]", dir))

cmd := exec.CommandContext(ctx, command[0], command[1:]...)
cmd.Dir = dir
cmd.Stderr = os.Stderr

stdout, err := cmd.StdoutPipe()
if err != nil {
defer cancel()
defer finished()
return nil, fmt.Errorf("StdoutPipe: %v", err)
}

if err = cmd.Start(); err != nil {
defer cancel()
defer finished()
_ = stdout.Close()
return nil, fmt.Errorf("Start: %v", err)
}

pid := process.GetManager().Add(fmt.Sprintf("GetBlame [repo_path: %s]", dir), cancel)

reader := bufio.NewReader(stdout)

return &BlameReader{
cmd,
pid,
stdout,
reader,
nil,
cancel,
cmd: cmd,
output: stdout,
reader: reader,
cancel: cancel,
finished: finished,
}, nil
}
4 changes: 2 additions & 2 deletions modules/git/blob_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type Blob struct {
// DataAsync gets a ReadCloser for the contents of a blob without reading it all.
// Calling the Close function on the result will discard all unread output.
func (b *Blob) DataAsync() (io.ReadCloser, error) {
wr, rd, cancel := b.repo.CatFileBatch()
wr, rd, cancel := b.repo.CatFileBatch(b.repo.Ctx)

_, err := wr.Write([]byte(b.ID.String() + "\n"))
if err != nil {
Expand Down Expand Up @@ -67,7 +67,7 @@ func (b *Blob) Size() int64 {
return b.size
}

wr, rd, cancel := b.repo.CatFileBatchCheck()
wr, rd, cancel := b.repo.CatFileBatchCheck(b.repo.Ctx)
defer cancel()
_, err := wr.Write([]byte(b.ID.String() + "\n"))
if err != nil {
Expand Down
16 changes: 7 additions & 9 deletions modules/git/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,13 @@ func (c *Command) RunWithContext(rc *RunContext) error {
log.Debug("%s: %v", rc.Dir, c)
}

ctx, cancel := context.WithTimeout(c.parentContext, rc.Timeout)
defer cancel()
desc := c.desc
if desc == "" {
desc = fmt.Sprintf("%s %s [repo_path: %s]", c.name, strings.Join(c.args, " "), rc.Dir)
}

ctx, cancel, finished := process.GetManager().AddContextTimeout(c.parentContext, rc.Timeout, desc)
defer finished()

cmd := exec.CommandContext(ctx, c.name, c.args...)
if rc.Env == nil {
Expand Down Expand Up @@ -172,13 +177,6 @@ func (c *Command) RunWithContext(rc *RunContext) error {
return err
}

desc := c.desc
if desc == "" {
desc = fmt.Sprintf("%s %s %s [repo_path: %s]", GitExecutable, c.name, strings.Join(c.args, " "), rc.Dir)
}
pid := process.GetManager().Add(desc, cancel)
defer process.GetManager().Remove(pid)

if rc.PipelineFunc != nil {
err := rc.PipelineFunc(ctx, cancel)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions modules/git/commit_info_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
}

func getLastCommitForPathsByCache(ctx context.Context, commitID, treePath string, paths []string, cache *LastCommitCache) (map[string]*Commit, []string, error) {
wr, rd, cancel := cache.repo.CatFileBatch()
wr, rd, cancel := cache.repo.CatFileBatch(ctx)
defer cancel()

var unHitEntryPaths []string
Expand Down Expand Up @@ -129,7 +129,7 @@ func GetLastCommitForPaths(ctx context.Context, cache *LastCommitCache, commit *
return nil, err
}

batchStdinWriter, batchReader, cancel := commit.repo.CatFileBatch()
batchStdinWriter, batchReader, cancel := commit.repo.CatFileBatch(ctx)
defer cancel()

commitsMap := map[string]*Commit{}
Expand Down
6 changes: 2 additions & 4 deletions modules/git/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ func GetRepoRawDiffForFile(repo *Repository, startCommit, endCommit string, diff
fileArgs = append(fileArgs, "--", file)
}
// FIXME: graceful: These commands should have a timeout
ctx, cancel := context.WithCancel(DefaultContext)
defer cancel()
ctx, _, finished := process.GetManager().AddContext(DefaultContext, fmt.Sprintf("GetRawDiffForFile: [repo_path: %s]", repo.Path))
defer finished()

var cmd *exec.Cmd
switch diffType {
Expand Down Expand Up @@ -90,8 +90,6 @@ func GetRepoRawDiffForFile(repo *Repository, startCommit, endCommit string, diff
cmd.Dir = repo.Path
cmd.Stdout = writer
cmd.Stderr = stderr
pid := process.GetManager().Add(fmt.Sprintf("GetRawDiffForFile: [repo_path: %s]", repo.Path), cancel)
defer process.GetManager().Remove(pid)

if err = cmd.Run(); err != nil {
return fmt.Errorf("Run: %v - %s", err, stderr)
Expand Down
2 changes: 1 addition & 1 deletion modules/git/pipeline/lfs_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) {

// Next feed the commits in order into cat-file --batch, followed by their trees and sub trees as necessary.
// so let's create a batch stdin and stdout
batchStdinWriter, batchReader, cancel := repo.CatFileBatch()
batchStdinWriter, batchReader, cancel := repo.CatFileBatch(repo.Ctx)
defer cancel()

// We'll use a scanner for the revList because it's simpler than a bufio.Reader
Expand Down
11 changes: 7 additions & 4 deletions modules/git/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,22 @@

package git

import "net/url"
import (
"context"
"net/url"
)

// GetRemoteAddress returns the url of a specific remote of the repository.
func GetRemoteAddress(repoPath, remoteName string) (*url.URL, error) {
func GetRemoteAddress(ctx context.Context, repoPath, remoteName string) (*url.URL, error) {
err := LoadGitVersion()
if err != nil {
return nil, err
}
var cmd *Command
if CheckGitVersionAtLeast("2.7") == nil {
cmd = NewCommand("remote", "get-url", remoteName)
cmd = NewCommandContext(ctx, "remote", "get-url", remoteName)
} else {
cmd = NewCommand("config", "--get", "remote."+remoteName+".url")
cmd = NewCommandContext(ctx, "config", "--get", "remote."+remoteName+".url")
}

result, err := cmd.RunInDir(repoPath)
Expand Down
4 changes: 2 additions & 2 deletions modules/git/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,8 @@ type PushOptions struct {
}

// Push pushs local commits to given remote branch.
func Push(repoPath string, opts PushOptions) error {
cmd := NewCommand("push")
func Push(ctx context.Context, repoPath string, opts PushOptions) error {
cmd := NewCommandContext(ctx, "push")
if opts.Force {
cmd.AddArguments("-f")
}
Expand Down
2 changes: 1 addition & 1 deletion modules/git/repo_attribute.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (repo *Repository) CheckAttribute(opts CheckAttributeOpts) (map[string]map[
}
}

cmd := NewCommand(cmdArgs...)
cmd := NewCommandContext(repo.Ctx, cmdArgs...)

if err := cmd.RunInDirTimeoutEnvPipeline(env, -1, repo.Path, stdOut, stdErr); err != nil {
return nil, fmt.Errorf("failed to run check-attr: %v\n%s\n%s", err, stdOut.String(), stdErr.String())
Expand Down
9 changes: 9 additions & 0 deletions modules/git/repo_base_gogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
package git

import (
"context"
"errors"
"path/filepath"

Expand All @@ -30,10 +31,17 @@ type Repository struct {
gogitRepo *gogit.Repository
gogitStorage *filesystem.Storage
gpgSettings *GPGSettings

Ctx context.Context
}

// OpenRepository opens the repository at the given path.
func OpenRepository(repoPath string) (*Repository, error) {
return OpenRepositoryCtx(DefaultContext, repoPath)
}

// OpenRepositoryCtx opens the repository at the given path within the context.Context
func OpenRepositoryCtx(ctx context.Context, repoPath string) (*Repository, error) {
repoPath, err := filepath.Abs(repoPath)
if err != nil {
return nil, err
Expand All @@ -60,6 +68,7 @@ func OpenRepository(repoPath string) (*Repository, error) {
gogitRepo: gogitRepo,
gogitStorage: storage,
tagCache: newObjectCache(),
Ctx: ctx,
}, nil
}

Expand Down
20 changes: 14 additions & 6 deletions modules/git/repo_base_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,17 @@ type Repository struct {
checkCancel context.CancelFunc
checkReader *bufio.Reader
checkWriter WriteCloserError

Ctx context.Context
}

// OpenRepository opens the repository at the given path.
func OpenRepository(repoPath string) (*Repository, error) {
return OpenRepositoryCtx(DefaultContext, repoPath)
}

// OpenRepositoryCtx opens the repository at the given path with the provided context.
func OpenRepositoryCtx(ctx context.Context, repoPath string) (*Repository, error) {
repoPath, err := filepath.Abs(repoPath)
if err != nil {
return nil, err
Expand All @@ -46,28 +53,29 @@ func OpenRepository(repoPath string) (*Repository, error) {
repo := &Repository{
Path: repoPath,
tagCache: newObjectCache(),
Ctx: ctx,
}

repo.batchWriter, repo.batchReader, repo.batchCancel = CatFileBatch(repoPath)
repo.checkWriter, repo.checkReader, repo.checkCancel = CatFileBatchCheck(repo.Path)
repo.batchWriter, repo.batchReader, repo.batchCancel = CatFileBatch(ctx, repoPath)
repo.checkWriter, repo.checkReader, repo.checkCancel = CatFileBatchCheck(ctx, repo.Path)

return repo, nil
}

// CatFileBatch obtains a CatFileBatch for this repository
func (repo *Repository) CatFileBatch() (WriteCloserError, *bufio.Reader, func()) {
func (repo *Repository) CatFileBatch(ctx context.Context) (WriteCloserError, *bufio.Reader, func()) {
if repo.batchCancel == nil || repo.batchReader.Buffered() > 0 {
log.Debug("Opening temporary cat file batch for: %s", repo.Path)
return CatFileBatch(repo.Path)
return CatFileBatch(ctx, repo.Path)
}
return repo.batchWriter, repo.batchReader, func() {}
}

// CatFileBatchCheck obtains a CatFileBatchCheck for this repository
func (repo *Repository) CatFileBatchCheck() (WriteCloserError, *bufio.Reader, func()) {
func (repo *Repository) CatFileBatchCheck(ctx context.Context) (WriteCloserError, *bufio.Reader, func()) {
if repo.checkCancel == nil || repo.checkReader.Buffered() > 0 {
log.Debug("Opening temporary cat file batch-check: %s", repo.Path)
return CatFileBatchCheck(repo.Path)
return CatFileBatchCheck(ctx, repo.Path)
}
return repo.checkWriter, repo.checkReader, func() {}
}
Expand Down
4 changes: 2 additions & 2 deletions modules/git/repo_blame.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import "fmt"

// FileBlame return the Blame object of file
func (repo *Repository) FileBlame(revision, path, file string) ([]byte, error) {
return NewCommand("blame", "--root", "--", file).RunInDirBytes(path)
return NewCommandContext(repo.Ctx, "blame", "--root", "--", file).RunInDirBytes(path)
}

// LineBlame returns the latest commit at the given line
func (repo *Repository) LineBlame(revision, path, file string, line uint) (*Commit, error) {
res, err := NewCommand("blame", fmt.Sprintf("-L %d,%d", line, line), "-p", revision, "--", file).RunInDir(path)
res, err := NewCommandContext(repo.Ctx, "blame", fmt.Sprintf("-L %d,%d", line, line), "-p", revision, "--", file).RunInDir(path)
if err != nil {
return nil, err
}
Expand Down
Loading