diff --git a/server/events/working_dir.go b/server/events/working_dir.go index 4ccc61fe6f..2cfeed8b8c 100644 --- a/server/events/working_dir.go +++ b/server/events/working_dir.go @@ -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 @@ -94,6 +95,16 @@ 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)) @@ -101,6 +112,13 @@ func (w *FileWorkspace) Clone( 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. @@ -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) }