Skip to content

Commit

Permalink
Reduce the number of git calls in parallel mode when merging
Browse files Browse the repository at this point in the history
All Clone() calls that have signaled an interest in merging
before another Clone() checks whether a merge is necessary
can skip their own checks.

This should reduce the thundering herd problem at the
beginning of large paralell runs.
  • Loading branch information
finnag committed Aug 16, 2023
1 parent 56d1a87 commit c5b3063
Showing 1 changed file with 26 additions and 4 deletions.
30 changes: 26 additions & 4 deletions server/events/working_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
const workingDirPrefix = "repos"

var cloneLocks sync.Map
var recheckRequiredMap sync.Map

//go:generate pegomock generate --package mocks -o mocks/mock_working_dir.go WorkingDir
//go:generate pegomock generate --package events WorkingDir
Expand Down Expand Up @@ -94,13 +95,30 @@ func (w *FileWorkspace) Clone(
mustRecheckUpstream bool) (string, bool, error) {
cloneDir := w.cloneDir(p.BaseRepo, p, workspace)

// Disable the mustRecheckUpstream flag if we are not using the merge checkout strategy
mustRecheckUpstream = mustRecheckUpstream && w.CheckoutMerge

if mustRecheckUpstream {
// We atomically set the recheckRequiredMap flag here before grabbing the clone lock.
// If the flag is cleared after we grab the lock, it means some other thread
// did the necessary work late enough that we do not have to do it again.
recheckRequiredMap.Store(cloneDir, struct{}{})
}

// Unconditionally wait for the clone lock here, if anyone else is doing any clone
// operation in this directory, we wait for it to finish before we check anything.
value, _ := cloneLocks.LoadOrStore(cloneDir, new(sync.Mutex))
mutex := value.(*sync.Mutex)
mutex.Lock()
defer mutex.Unlock()

if mustRecheckUpstream {
if _, exists := recheckRequiredMap.Load(cloneDir); !exists {
mustRecheckUpstream = false
w.Logger.Debug("Skipping upstream check. Some other thread has done this for us")
}
}

c := wrappedGitContext{cloneDir, headRepo, p}
// If the directory already exists, check if it's at the right commit.
// If so, then we do nothing.
Expand Down Expand Up @@ -128,13 +146,17 @@ func (w *FileWorkspace) Clone(
// We're prefix matching here because BitBucket doesn't give us the full
// commit, only a 12 character prefix.
if strings.HasPrefix(currCommit, p.HeadCommit) {
if mustRecheckUpstream && w.CheckoutMerge && w.recheckDiverged(p, headRepo, cloneDir) {
w.Logger.Info("base branch has been updated, using merge strategy and will clone again")
return cloneDir, true, w.mergeAgain(c)
if mustRecheckUpstream {
recheckRequiredMap.Delete(cloneDir)
if w.recheckDiverged(p, headRepo, cloneDir) {
w.Logger.Info("base branch has been updated, using merge strategy and will merge again")
return cloneDir, true, w.mergeAgain(c)
}
w.Logger.Debug("repo is at correct commit %q and there are no upstream changes", p.HeadCommit)
} else {
w.Logger.Debug("repo is at correct commit %q so will not re-clone", p.HeadCommit)
return cloneDir, false, nil
}
return cloneDir, false, nil
} else {
w.Logger.Debug("repo was already cloned but is not at correct commit, wanted %q got %q", p.HeadCommit, currCommit)
}
Expand Down

0 comments on commit c5b3063

Please sign in to comment.