gitserver: Don't trigger clones in hot requests (#61672)

This removes the enqueueing of clones during git API requests for all but dotcom.
This didn't serve a good purpose, the repos are all expected to be cloned in a steady state, and uncloned repos are regularly prioritized for cloning.
Cloning repos out-of-band in here caused large spikes in clone jobs on dotcom occasionally.
Also, since the queue was in-memory, we lost those requests across restarts.

Since for sourcegraph.com, we don't manage all repos by the scheduler, we have to keep some path here, and we do that now by asking the repo updater scheduler to consider a clone (through repo update) for that repo, but on the priority and concurrency of the repo fetch scheduler instead of out of band now.

This will help make the scheduler fully own the decision making about fetches and clones.

Test plan:

E2E and integration test suites still work, adjusted the clone test and it's still passing.

cmd/gitserver/internal/BUILD.bazel

@@ -6,7 +6,6 @@ go_library(
     name = "internal",
     srcs = [
         "cleanup.go",
-        "clone.go",
         "ensurerevision.go",
         "gitservice.go",
         "list_gitolite.go",
@@ -61,6 +60,7 @@ go_library(
         "//internal/observation",
         "//internal/perforce",
         "//internal/ratelimit",
+        "//internal/repoupdater",
         "//internal/security",
         "//internal/trace",
         "//internal/types",

cmd/gitserver/internal/clone.go

@@ -1,47 +0,0 @@
-package internal
-
-import (
-	"context"
-
-	"github.com/sourcegraph/log"
-
-	"github.com/sourcegraph/sourcegraph/internal/api"
-	"github.com/sourcegraph/sourcegraph/internal/conf"
-	"github.com/sourcegraph/sourcegraph/lib/errors"
-)
-
-type CloneStatus struct {
-	CloneInProgress bool
-	CloneProgress   string
-}
-
-// MaybeStartClone checks if a given repository is cloned on disk. If not, it starts
-// cloning the repository in the background and returns a CloneStatus.
-// Note: If disableAutoGitUpdates is set in the site config, no operation is taken and
-// a NotFound error is returned.
-func (s *Server) MaybeStartClone(ctx context.Context, repo api.RepoName) (cloned bool, status CloneStatus, _ error) {
-	cloned, err := s.fs.RepoCloned(repo)
-	if err != nil {
-		return false, CloneStatus{}, errors.Wrap(err, "determine clone status")
-	}
-
-	if cloned {
-		return true, CloneStatus{}, nil
-	}
-
-	if conf.Get().DisableAutoGitUpdates {
-		s.logger.Debug("not cloning on demand as DisableAutoGitUpdates is set")
-		return false, CloneStatus{}, nil
-	}
-
-	cloneProgress, err := s.CloneRepo(ctx, repo, CloneOptions{})
-	if err != nil {
-		s.logger.Warn("error starting repo clone", log.String("repo", string(repo)), log.Error(err))
-		return false, CloneStatus{}, nil
-	}
-
-	return false, CloneStatus{
-		CloneInProgress: true,
-		CloneProgress:   cloneProgress,
-	}, nil
-}

cmd/gitserver/internal/mocks_test.go

@@ -16,6 +16,652 @@ import (
 	trace "github.com/sourcegraph/sourcegraph/internal/trace"
 )
 
+// MockRepositoryLock is a mock implementation of the RepositoryLock
+// interface (from the package
+// github.com/sourcegraph/sourcegraph/cmd/gitserver/internal) used for unit
+// testing.
+type MockRepositoryLock struct {
+	// ReleaseFunc is an instance of a mock function object controlling the
+	// behavior of the method Release.
+	ReleaseFunc *RepositoryLockReleaseFunc
+	// SetStatusFunc is an instance of a mock function object controlling
+	// the behavior of the method SetStatus.
+	SetStatusFunc *RepositoryLockSetStatusFunc
+}
+
+// NewMockRepositoryLock creates a new mock of the RepositoryLock interface.
+// All methods return zero values for all results, unless overwritten.
+func NewMockRepositoryLock() *MockRepositoryLock {
+	return &MockRepositoryLock{
+		ReleaseFunc: &RepositoryLockReleaseFunc{
+			defaultHook: func() {
+				return
+			},
+		},
+		SetStatusFunc: &RepositoryLockSetStatusFunc{
+			defaultHook: func(string) {
+				return
+			},
+		},
+	}
+}
+
+// NewStrictMockRepositoryLock creates a new mock of the RepositoryLock
+// interface. All methods panic on invocation, unless overwritten.
+func NewStrictMockRepositoryLock() *MockRepositoryLock {
+	return &MockRepositoryLock{
+		ReleaseFunc: &RepositoryLockReleaseFunc{
+			defaultHook: func() {
+				panic("unexpected invocation of MockRepositoryLock.Release")
+			},
+		},
+		SetStatusFunc: &RepositoryLockSetStatusFunc{
+			defaultHook: func(string) {
+				panic("unexpected invocation of MockRepositoryLock.SetStatus")
+			},
+		},
+	}
+}
+
+// NewMockRepositoryLockFrom creates a new mock of the MockRepositoryLock
+// interface. All methods delegate to the given implementation, unless
+// overwritten.
+func NewMockRepositoryLockFrom(i RepositoryLock) *MockRepositoryLock {
+	return &MockRepositoryLock{
+		ReleaseFunc: &RepositoryLockReleaseFunc{
+			defaultHook: i.Release,
+		},
+		SetStatusFunc: &RepositoryLockSetStatusFunc{
+			defaultHook: i.SetStatus,
+		},
+	}
+}
+
+// RepositoryLockReleaseFunc describes the behavior when the Release method
+// of the parent MockRepositoryLock instance is invoked.
+type RepositoryLockReleaseFunc struct {
+	defaultHook func()
+	hooks       []func()
+	history     []RepositoryLockReleaseFuncCall
+	mutex       sync.Mutex
+}
+
+// Release delegates to the next hook function in the queue and stores the
+// parameter and result values of this invocation.
+func (m *MockRepositoryLock) Release() {
+	m.ReleaseFunc.nextHook()()
+	m.ReleaseFunc.appendCall(RepositoryLockReleaseFuncCall{})
+	return
+}
+
+// SetDefaultHook sets function that is called when the Release method of
+// the parent MockRepositoryLock instance is invoked and the hook queue is
+// empty.
+func (f *RepositoryLockReleaseFunc) SetDefaultHook(hook func()) {
+	f.defaultHook = hook
+}
+
+// PushHook adds a function to the end of hook queue. Each invocation of the
+// Release method of the parent MockRepositoryLock instance invokes the hook
+// at the front of the queue and discards it. After the queue is empty, the
+// default hook function is invoked for any future action.
+func (f *RepositoryLockReleaseFunc) PushHook(hook func()) {
+	f.mutex.Lock()
+	f.hooks = append(f.hooks, hook)
+	f.mutex.Unlock()
+}
+
+// SetDefaultReturn calls SetDefaultHook with a function that returns the
+// given values.
+func (f *RepositoryLockReleaseFunc) SetDefaultReturn() {
+	f.SetDefaultHook(func() {
+		return
+	})
+}
+
+// PushReturn calls PushHook with a function that returns the given values.
+func (f *RepositoryLockReleaseFunc) PushReturn() {
+	f.PushHook(func() {
+		return
+	})
+}
+
+func (f *RepositoryLockReleaseFunc) nextHook() func() {
+	f.mutex.Lock()
+	defer f.mutex.Unlock()
+
+	if len(f.hooks) == 0 {
+		return f.defaultHook
+	}
+
+	hook := f.hooks[0]
+	f.hooks = f.hooks[1:]
+	return hook
+}
+
+func (f *RepositoryLockReleaseFunc) appendCall(r0 RepositoryLockReleaseFuncCall) {
+	f.mutex.Lock()
+	f.history = append(f.history, r0)
+	f.mutex.Unlock()
+}
+
+// History returns a sequence of RepositoryLockReleaseFuncCall objects
+// describing the invocations of this function.
+func (f *RepositoryLockReleaseFunc) History() []RepositoryLockReleaseFuncCall {
+	f.mutex.Lock()
+	history := make([]RepositoryLockReleaseFuncCall, len(f.history))
+	copy(history, f.history)
+	f.mutex.Unlock()
+
+	return history
+}
+
+// RepositoryLockReleaseFuncCall is an object that describes an invocation
+// of method Release on an instance of MockRepositoryLock.
+type RepositoryLockReleaseFuncCall struct{}
+
+// Args returns an interface slice containing the arguments of this
+// invocation.
+func (c RepositoryLockReleaseFuncCall) Args() []interface{} {
+	return []interface{}{}
+}
+
+// Results returns an interface slice containing the results of this
+// invocation.
+func (c RepositoryLockReleaseFuncCall) Results() []interface{} {
+	return []interface{}{}
+}
+
+// RepositoryLockSetStatusFunc describes the behavior when the SetStatus
+// method of the parent MockRepositoryLock instance is invoked.
+type RepositoryLockSetStatusFunc struct {
+	defaultHook func(string)
+	hooks       []func(string)
+	history     []RepositoryLockSetStatusFuncCall
+	mutex       sync.Mutex
+}
+
+// SetStatus delegates to the next hook function in the queue and stores the
+// parameter and result values of this invocation.
+func (m *MockRepositoryLock) SetStatus(v0 string) {
+	m.SetStatusFunc.nextHook()(v0)
+	m.SetStatusFunc.appendCall(RepositoryLockSetStatusFuncCall{v0})
+	return
+}
+
+// SetDefaultHook sets function that is called when the SetStatus method of
+// the parent MockRepositoryLock instance is invoked and the hook queue is
+// empty.
+func (f *RepositoryLockSetStatusFunc) SetDefaultHook(hook func(string)) {
+	f.defaultHook = hook
+}
+
+// PushHook adds a function to the end of hook queue. Each invocation of the
+// SetStatus method of the parent MockRepositoryLock instance invokes the
+// hook at the front of the queue and discards it. After the queue is empty,
+// the default hook function is invoked for any future action.
+func (f *RepositoryLockSetStatusFunc) PushHook(hook func(string)) {
+	f.mutex.Lock()
+	f.hooks = append(f.hooks, hook)
+	f.mutex.Unlock()
+}
+
+// SetDefaultReturn calls SetDefaultHook with a function that returns the
+// given values.
+func (f *RepositoryLockSetStatusFunc) SetDefaultReturn() {
+	f.SetDefaultHook(func(string) {
+		return
+	})
+}
+
+// PushReturn calls PushHook with a function that returns the given values.
+func (f *RepositoryLockSetStatusFunc) PushReturn() {
+	f.PushHook(func(string) {
+		return
+	})
+}
+
+func (f *RepositoryLockSetStatusFunc) nextHook() func(string) {
+	f.mutex.Lock()
+	defer f.mutex.Unlock()
+
+	if len(f.hooks) == 0 {
+		return f.defaultHook
+	}
+
+	hook := f.hooks[0]
+	f.hooks = f.hooks[1:]
+	return hook
+}
+
+func (f *RepositoryLockSetStatusFunc) appendCall(r0 RepositoryLockSetStatusFuncCall) {
+	f.mutex.Lock()
+	f.history = append(f.history, r0)
+	f.mutex.Unlock()
+}
+
+// History returns a sequence of RepositoryLockSetStatusFuncCall objects
+// describing the invocations of this function.
+func (f *RepositoryLockSetStatusFunc) History() []RepositoryLockSetStatusFuncCall {
+	f.mutex.Lock()
+	history := make([]RepositoryLockSetStatusFuncCall, len(f.history))
+	copy(history, f.history)
+	f.mutex.Unlock()
+
+	return history
+}
+
+// RepositoryLockSetStatusFuncCall is an object that describes an invocation
+// of method SetStatus on an instance of MockRepositoryLock.
+type RepositoryLockSetStatusFuncCall struct {
+	// Arg0 is the value of the 1st argument passed to this method
+	// invocation.
+	Arg0 string
+}
+
+// Args returns an interface slice containing the arguments of this
+// invocation.
+func (c RepositoryLockSetStatusFuncCall) Args() []interface{} {
+	return []interface{}{c.Arg0}
+}
+
+// Results returns an interface slice containing the results of this
+// invocation.
+func (c RepositoryLockSetStatusFuncCall) Results() []interface{} {
+	return []interface{}{}
+}
+
+// MockRepositoryLocker is a mock implementation of the RepositoryLocker
+// interface (from the package
+// github.com/sourcegraph/sourcegraph/cmd/gitserver/internal) used for unit
+// testing.
+type MockRepositoryLocker struct {
+	// AllStatusesFunc is an instance of a mock function object controlling
+	// the behavior of the method AllStatuses.
+	AllStatusesFunc *RepositoryLockerAllStatusesFunc
+	// StatusFunc is an instance of a mock function object controlling the
+	// behavior of the method Status.
+	StatusFunc *RepositoryLockerStatusFunc
+	// TryAcquireFunc is an instance of a mock function object controlling
+	// the behavior of the method TryAcquire.
+	TryAcquireFunc *RepositoryLockerTryAcquireFunc
+}
+
+// NewMockRepositoryLocker creates a new mock of the RepositoryLocker
+// interface. All methods return zero values for all results, unless
+// overwritten.
+func NewMockRepositoryLocker() *MockRepositoryLocker {
+	return &MockRepositoryLocker{
+		AllStatusesFunc: &RepositoryLockerAllStatusesFunc{
+			defaultHook: func() (r0 map[api.RepoName]string) {
+				return
+			},
+		},
+		StatusFunc: &RepositoryLockerStatusFunc{
+			defaultHook: func(api.RepoName) (r0 string, r1 bool) {
+				return
+			},
+		},
+		TryAcquireFunc: &RepositoryLockerTryAcquireFunc{
+			defaultHook: func(api.RepoName, string) (r0 RepositoryLock, r1 bool) {
+				return
+			},
+		},
+	}
+}
+
+// NewStrictMockRepositoryLocker creates a new mock of the RepositoryLocker
+// interface. All methods panic on invocation, unless overwritten.
+func NewStrictMockRepositoryLocker() *MockRepositoryLocker {
+	return &MockRepositoryLocker{
+		AllStatusesFunc: &RepositoryLockerAllStatusesFunc{
+			defaultHook: func() map[api.RepoName]string {
+				panic("unexpected invocation of MockRepositoryLocker.AllStatuses")
+			},
+		},
+		StatusFunc: &RepositoryLockerStatusFunc{
+			defaultHook: func(api.RepoName) (string, bool) {
+				panic("unexpected invocation of MockRepositoryLocker.Status")
+			},
+		},
+		TryAcquireFunc: &RepositoryLockerTryAcquireFunc{
+			defaultHook: func(api.RepoName, string) (RepositoryLock, bool) {
+				panic("unexpected invocation of MockRepositoryLocker.TryAcquire")
+			},
+		},
+	}
+}
+
+// NewMockRepositoryLockerFrom creates a new mock of the
+// MockRepositoryLocker interface. All methods delegate to the given
+// implementation, unless overwritten.
+func NewMockRepositoryLockerFrom(i RepositoryLocker) *MockRepositoryLocker {
+	return &MockRepositoryLocker{
+		AllStatusesFunc: &RepositoryLockerAllStatusesFunc{
+			defaultHook: i.AllStatuses,
+		},
+		StatusFunc: &RepositoryLockerStatusFunc{
+			defaultHook: i.Status,
+		},
+		TryAcquireFunc: &RepositoryLockerTryAcquireFunc{
+			defaultHook: i.TryAcquire,
+		},
+	}
+}
+
+// RepositoryLockerAllStatusesFunc describes the behavior when the
+// AllStatuses method of the parent MockRepositoryLocker instance is
+// invoked.
+type RepositoryLockerAllStatusesFunc struct {
+	defaultHook func() map[api.RepoName]string
+	hooks       []func() map[api.RepoName]string
+	history     []RepositoryLockerAllStatusesFuncCall
+	mutex       sync.Mutex
+}
+
+// AllStatuses delegates to the next hook function in the queue and stores
+// the parameter and result values of this invocation.
+func (m *MockRepositoryLocker) AllStatuses() map[api.RepoName]string {
+	r0 := m.AllStatusesFunc.nextHook()()
+	m.AllStatusesFunc.appendCall(RepositoryLockerAllStatusesFuncCall{r0})
+	return r0
+}
+
+// SetDefaultHook sets function that is called when the AllStatuses method
+// of the parent MockRepositoryLocker instance is invoked and the hook queue
+// is empty.
+func (f *RepositoryLockerAllStatusesFunc) SetDefaultHook(hook func() map[api.RepoName]string) {
+	f.defaultHook = hook
+}
+
+// PushHook adds a function to the end of hook queue. Each invocation of the
+// AllStatuses method of the parent MockRepositoryLocker instance invokes
+// the hook at the front of the queue and discards it. After the queue is
+// empty, the default hook function is invoked for any future action.
+func (f *RepositoryLockerAllStatusesFunc) PushHook(hook func() map[api.RepoName]string) {
+	f.mutex.Lock()
+	f.hooks = append(f.hooks, hook)
+	f.mutex.Unlock()
+}
+
+// SetDefaultReturn calls SetDefaultHook with a function that returns the
+// given values.
+func (f *RepositoryLockerAllStatusesFunc) SetDefaultReturn(r0 map[api.RepoName]string) {
+	f.SetDefaultHook(func() map[api.RepoName]string {
+		return r0
+	})
+}
+
+// PushReturn calls PushHook with a function that returns the given values.
+func (f *RepositoryLockerAllStatusesFunc) PushReturn(r0 map[api.RepoName]string) {
+	f.PushHook(func() map[api.RepoName]string {
+		return r0
+	})
+}
+
+func (f *RepositoryLockerAllStatusesFunc) nextHook() func() map[api.RepoName]string {
+	f.mutex.Lock()
+	defer f.mutex.Unlock()
+
+	if len(f.hooks) == 0 {
+		return f.defaultHook
+	}
+
+	hook := f.hooks[0]
+	f.hooks = f.hooks[1:]
+	return hook
+}
+
+func (f *RepositoryLockerAllStatusesFunc) appendCall(r0 RepositoryLockerAllStatusesFuncCall) {
+	f.mutex.Lock()
+	f.history = append(f.history, r0)
+	f.mutex.Unlock()
+}
+
+// History returns a sequence of RepositoryLockerAllStatusesFuncCall objects
+// describing the invocations of this function.
+func (f *RepositoryLockerAllStatusesFunc) History() []RepositoryLockerAllStatusesFuncCall {
+	f.mutex.Lock()
+	history := make([]RepositoryLockerAllStatusesFuncCall, len(f.history))
+	copy(history, f.history)
+	f.mutex.Unlock()
+
+	return history
+}
+
+// RepositoryLockerAllStatusesFuncCall is an object that describes an
+// invocation of method AllStatuses on an instance of MockRepositoryLocker.
+type RepositoryLockerAllStatusesFuncCall struct {
+	// Result0 is the value of the 1st result returned from this method
+	// invocation.
+	Result0 map[api.RepoName]string
+}
+
+// Args returns an interface slice containing the arguments of this
+// invocation.
+func (c RepositoryLockerAllStatusesFuncCall) Args() []interface{} {
+	return []interface{}{}
+}
+
+// Results returns an interface slice containing the results of this
+// invocation.
+func (c RepositoryLockerAllStatusesFuncCall) Results() []interface{} {
+	return []interface{}{c.Result0}
+}
+
+// RepositoryLockerStatusFunc describes the behavior when the Status method
+// of the parent MockRepositoryLocker instance is invoked.
+type RepositoryLockerStatusFunc struct {
+	defaultHook func(api.RepoName) (string, bool)
+	hooks       []func(api.RepoName) (string, bool)
+	history     []RepositoryLockerStatusFuncCall
+	mutex       sync.Mutex
+}
+
+// Status delegates to the next hook function in the queue and stores the
+// parameter and result values of this invocation.
+func (m *MockRepositoryLocker) Status(v0 api.RepoName) (string, bool) {
+	r0, r1 := m.StatusFunc.nextHook()(v0)
+	m.StatusFunc.appendCall(RepositoryLockerStatusFuncCall{v0, r0, r1})
+	return r0, r1
+}
+
+// SetDefaultHook sets function that is called when the Status method of the
+// parent MockRepositoryLocker instance is invoked and the hook queue is
+// empty.
+func (f *RepositoryLockerStatusFunc) SetDefaultHook(hook func(api.RepoName) (string, bool)) {
+	f.defaultHook = hook
+}
+
+// PushHook adds a function to the end of hook queue. Each invocation of the
+// Status method of the parent MockRepositoryLocker instance invokes the
+// hook at the front of the queue and discards it. After the queue is empty,
+// the default hook function is invoked for any future action.
+func (f *RepositoryLockerStatusFunc) PushHook(hook func(api.RepoName) (string, bool)) {
+	f.mutex.Lock()
+	f.hooks = append(f.hooks, hook)
+	f.mutex.Unlock()
+}
+
+// SetDefaultReturn calls SetDefaultHook with a function that returns the
+// given values.
+func (f *RepositoryLockerStatusFunc) SetDefaultReturn(r0 string, r1 bool) {
+	f.SetDefaultHook(func(api.RepoName) (string, bool) {
+		return r0, r1
+	})
+}
+
+// PushReturn calls PushHook with a function that returns the given values.
+func (f *RepositoryLockerStatusFunc) PushReturn(r0 string, r1 bool) {
+	f.PushHook(func(api.RepoName) (string, bool) {
+		return r0, r1
+	})
+}
+
+func (f *RepositoryLockerStatusFunc) nextHook() func(api.RepoName) (string, bool) {
+	f.mutex.Lock()
+	defer f.mutex.Unlock()
+
+	if len(f.hooks) == 0 {
+		return f.defaultHook
+	}
+
+	hook := f.hooks[0]
+	f.hooks = f.hooks[1:]
+	return hook
+}
+
+func (f *RepositoryLockerStatusFunc) appendCall(r0 RepositoryLockerStatusFuncCall) {
+	f.mutex.Lock()
+	f.history = append(f.history, r0)
+	f.mutex.Unlock()
+}
+
+// History returns a sequence of RepositoryLockerStatusFuncCall objects
+// describing the invocations of this function.
+func (f *RepositoryLockerStatusFunc) History() []RepositoryLockerStatusFuncCall {
+	f.mutex.Lock()
+	history := make([]RepositoryLockerStatusFuncCall, len(f.history))
+	copy(history, f.history)
+	f.mutex.Unlock()
+
+	return history
+}
+
+// RepositoryLockerStatusFuncCall is an object that describes an invocation
+// of method Status on an instance of MockRepositoryLocker.
+type RepositoryLockerStatusFuncCall struct {
+	// Arg0 is the value of the 1st argument passed to this method
+	// invocation.
+	Arg0 api.RepoName
+	// Result0 is the value of the 1st result returned from this method
+	// invocation.
+	Result0 string
+	// Result1 is the value of the 2nd result returned from this method
+	// invocation.
+	Result1 bool
+}
+
+// Args returns an interface slice containing the arguments of this
+// invocation.
+func (c RepositoryLockerStatusFuncCall) Args() []interface{} {
+	return []interface{}{c.Arg0}
+}
+
+// Results returns an interface slice containing the results of this
+// invocation.
+func (c RepositoryLockerStatusFuncCall) Results() []interface{} {
+	return []interface{}{c.Result0, c.Result1}
+}
+
+// RepositoryLockerTryAcquireFunc describes the behavior when the TryAcquire
+// method of the parent MockRepositoryLocker instance is invoked.
+type RepositoryLockerTryAcquireFunc struct {
+	defaultHook func(api.RepoName, string) (RepositoryLock, bool)
+	hooks       []func(api.RepoName, string) (RepositoryLock, bool)
+	history     []RepositoryLockerTryAcquireFuncCall
+	mutex       sync.Mutex
+}
+
+// TryAcquire delegates to the next hook function in the queue and stores
+// the parameter and result values of this invocation.
+func (m *MockRepositoryLocker) TryAcquire(v0 api.RepoName, v1 string) (RepositoryLock, bool) {
+	r0, r1 := m.TryAcquireFunc.nextHook()(v0, v1)
+	m.TryAcquireFunc.appendCall(RepositoryLockerTryAcquireFuncCall{v0, v1, r0, r1})
+	return r0, r1
+}
+
+// SetDefaultHook sets function that is called when the TryAcquire method of
+// the parent MockRepositoryLocker instance is invoked and the hook queue is
+// empty.
+func (f *RepositoryLockerTryAcquireFunc) SetDefaultHook(hook func(api.RepoName, string) (RepositoryLock, bool)) {
+	f.defaultHook = hook
+}
+
+// PushHook adds a function to the end of hook queue. Each invocation of the
+// TryAcquire method of the parent MockRepositoryLocker instance invokes the
+// hook at the front of the queue and discards it. After the queue is empty,
+// the default hook function is invoked for any future action.
+func (f *RepositoryLockerTryAcquireFunc) PushHook(hook func(api.RepoName, string) (RepositoryLock, bool)) {
+	f.mutex.Lock()
+	f.hooks = append(f.hooks, hook)
+	f.mutex.Unlock()
+}
+
+// SetDefaultReturn calls SetDefaultHook with a function that returns the
+// given values.
+func (f *RepositoryLockerTryAcquireFunc) SetDefaultReturn(r0 RepositoryLock, r1 bool) {
+	f.SetDefaultHook(func(api.RepoName, string) (RepositoryLock, bool) {
+		return r0, r1
+	})
+}
+
+// PushReturn calls PushHook with a function that returns the given values.
+func (f *RepositoryLockerTryAcquireFunc) PushReturn(r0 RepositoryLock, r1 bool) {
+	f.PushHook(func(api.RepoName, string) (RepositoryLock, bool) {
+		return r0, r1
+	})
+}
+
+func (f *RepositoryLockerTryAcquireFunc) nextHook() func(api.RepoName, string) (RepositoryLock, bool) {
+	f.mutex.Lock()
+	defer f.mutex.Unlock()
+
+	if len(f.hooks) == 0 {
+		return f.defaultHook
+	}
+
+	hook := f.hooks[0]
+	f.hooks = f.hooks[1:]
+	return hook
+}
+
+func (f *RepositoryLockerTryAcquireFunc) appendCall(r0 RepositoryLockerTryAcquireFuncCall) {
+	f.mutex.Lock()
+	f.history = append(f.history, r0)
+	f.mutex.Unlock()
+}
+
+// History returns a sequence of RepositoryLockerTryAcquireFuncCall objects
+// describing the invocations of this function.
+func (f *RepositoryLockerTryAcquireFunc) History() []RepositoryLockerTryAcquireFuncCall {
+	f.mutex.Lock()
+	history := make([]RepositoryLockerTryAcquireFuncCall, len(f.history))
+	copy(history, f.history)
+	f.mutex.Unlock()
+
+	return history
+}
+
+// RepositoryLockerTryAcquireFuncCall is an object that describes an
+// invocation of method TryAcquire on an instance of MockRepositoryLocker.
+type RepositoryLockerTryAcquireFuncCall struct {
+	// Arg0 is the value of the 1st argument passed to this method
+	// invocation.
+	Arg0 api.RepoName
+	// Arg1 is the value of the 2nd argument passed to this method
+	// invocation.
+	Arg1 string
+	// Result0 is the value of the 1st result returned from this method
+	// invocation.
+	Result0 RepositoryLock
+	// Result1 is the value of the 2nd result returned from this method
+	// invocation.
+	Result1 bool
+}
+
+// Args returns an interface slice containing the arguments of this
+// invocation.
+func (c RepositoryLockerTryAcquireFuncCall) Args() []interface{} {
+	return []interface{}{c.Arg0, c.Arg1}
+}
+
+// Results returns an interface slice containing the results of this
+// invocation.
+func (c RepositoryLockerTryAcquireFuncCall) Results() []interface{} {
+	return []interface{}{c.Result0, c.Result1}
+}
+
 // MockService is a mock implementation of the service interface (from the
 // package github.com/sourcegraph/sourcegraph/cmd/gitserver/internal) used
 // for unit testing.
@@ -32,9 +678,6 @@ type MockService struct {
 	// LogIfCorruptFunc is an instance of a mock function object controlling
 	// the behavior of the method LogIfCorrupt.
 	LogIfCorruptFunc *ServiceLogIfCorruptFunc
-	// MaybeStartCloneFunc is an instance of a mock function object
-	// controlling the behavior of the method MaybeStartClone.
-	MaybeStartCloneFunc *ServiceMaybeStartCloneFunc
 	// RepoUpdateFunc is an instance of a mock function object controlling
 	// the behavior of the method RepoUpdate.
 	RepoUpdateFunc *ServiceRepoUpdateFunc
@@ -67,11 +710,6 @@ func NewMockService() *MockService {
 				return
 			},
 		},
-		MaybeStartCloneFunc: &ServiceMaybeStartCloneFunc{
-			defaultHook: func(context.Context, api.RepoName) (r0 bool, r1 CloneStatus, r2 error) {
-				return
-			},
-		},
 		RepoUpdateFunc: &ServiceRepoUpdateFunc{
 			defaultHook: func(context.Context, *protocol.RepoUpdateRequest) (r0 protocol.RepoUpdateResponse) {
 				return
@@ -109,11 +747,6 @@ func NewStrictMockService() *MockService {
 				panic("unexpected invocation of MockService.LogIfCorrupt")
 			},
 		},
-		MaybeStartCloneFunc: &ServiceMaybeStartCloneFunc{
-			defaultHook: func(context.Context, api.RepoName) (bool, CloneStatus, error) {
-				panic("unexpected invocation of MockService.MaybeStartClone")
-			},
-		},
 		RepoUpdateFunc: &ServiceRepoUpdateFunc{
 			defaultHook: func(context.Context, *protocol.RepoUpdateRequest) protocol.RepoUpdateResponse {
 				panic("unexpected invocation of MockService.RepoUpdate")
@@ -135,7 +768,6 @@ type surrogateMockService interface {
 	EnsureRevision(context.Context, api.RepoName, string) bool
 	IsRepoCloneable(context.Context, api.RepoName) (protocol.IsRepoCloneableResponse, error)
 	LogIfCorrupt(context.Context, api.RepoName, error)
-	MaybeStartClone(context.Context, api.RepoName) (bool, CloneStatus, error)
 	RepoUpdate(context.Context, *protocol.RepoUpdateRequest) protocol.RepoUpdateResponse
 	SearchWithObservability(context.Context, trace.Trace, *protocol.SearchRequest, func(*protocol.CommitMatch) error) (bool, error)
 }
@@ -156,9 +788,6 @@ func NewMockServiceFrom(i surrogateMockService) *MockService {
 		LogIfCorruptFunc: &ServiceLogIfCorruptFunc{
 			defaultHook: i.LogIfCorrupt,
 		},
-		MaybeStartCloneFunc: &ServiceMaybeStartCloneFunc{
-			defaultHook: i.MaybeStartClone,
-		},
 		RepoUpdateFunc: &ServiceRepoUpdateFunc{
 			defaultHook: i.RepoUpdate,
 		},
@@ -598,117 +1227,6 @@ func (c ServiceLogIfCorruptFuncCall) Results() []interface{} {
 	return []interface{}{}
 }
 
-// ServiceMaybeStartCloneFunc describes the behavior when the
-// MaybeStartClone method of the parent MockService instance is invoked.
-type ServiceMaybeStartCloneFunc struct {
-	defaultHook func(context.Context, api.RepoName) (bool, CloneStatus, error)
-	hooks       []func(context.Context, api.RepoName) (bool, CloneStatus, error)
-	history     []ServiceMaybeStartCloneFuncCall
-	mutex       sync.Mutex
-}
-
-// MaybeStartClone delegates to the next hook function in the queue and
-// stores the parameter and result values of this invocation.
-func (m *MockService) MaybeStartClone(v0 context.Context, v1 api.RepoName) (bool, CloneStatus, error) {
-	r0, r1, r2 := m.MaybeStartCloneFunc.nextHook()(v0, v1)
-	m.MaybeStartCloneFunc.appendCall(ServiceMaybeStartCloneFuncCall{v0, v1, r0, r1, r2})
-	return r0, r1, r2
-}
-
-// SetDefaultHook sets function that is called when the MaybeStartClone
-// method of the parent MockService instance is invoked and the hook queue
-// is empty.
-func (f *ServiceMaybeStartCloneFunc) SetDefaultHook(hook func(context.Context, api.RepoName) (bool, CloneStatus, error)) {
-	f.defaultHook = hook
-}
-
-// PushHook adds a function to the end of hook queue. Each invocation of the
-// MaybeStartClone method of the parent MockService instance invokes the
-// hook at the front of the queue and discards it. After the queue is empty,
-// the default hook function is invoked for any future action.
-func (f *ServiceMaybeStartCloneFunc) PushHook(hook func(context.Context, api.RepoName) (bool, CloneStatus, error)) {
-	f.mutex.Lock()
-	f.hooks = append(f.hooks, hook)
-	f.mutex.Unlock()
-}
-
-// SetDefaultReturn calls SetDefaultHook with a function that returns the
-// given values.
-func (f *ServiceMaybeStartCloneFunc) SetDefaultReturn(r0 bool, r1 CloneStatus, r2 error) {
-	f.SetDefaultHook(func(context.Context, api.RepoName) (bool, CloneStatus, error) {
-		return r0, r1, r2
-	})
-}
-
-// PushReturn calls PushHook with a function that returns the given values.
-func (f *ServiceMaybeStartCloneFunc) PushReturn(r0 bool, r1 CloneStatus, r2 error) {
-	f.PushHook(func(context.Context, api.RepoName) (bool, CloneStatus, error) {
-		return r0, r1, r2
-	})
-}
-
-func (f *ServiceMaybeStartCloneFunc) nextHook() func(context.Context, api.RepoName) (bool, CloneStatus, error) {
-	f.mutex.Lock()
-	defer f.mutex.Unlock()
-
-	if len(f.hooks) == 0 {
-		return f.defaultHook
-	}
-
-	hook := f.hooks[0]
-	f.hooks = f.hooks[1:]
-	return hook
-}
-
-func (f *ServiceMaybeStartCloneFunc) appendCall(r0 ServiceMaybeStartCloneFuncCall) {
-	f.mutex.Lock()
-	f.history = append(f.history, r0)
-	f.mutex.Unlock()
-}
-
-// History returns a sequence of ServiceMaybeStartCloneFuncCall objects
-// describing the invocations of this function.
-func (f *ServiceMaybeStartCloneFunc) History() []ServiceMaybeStartCloneFuncCall {
-	f.mutex.Lock()
-	history := make([]ServiceMaybeStartCloneFuncCall, len(f.history))
-	copy(history, f.history)
-	f.mutex.Unlock()
-
-	return history
-}
-
-// ServiceMaybeStartCloneFuncCall is an object that describes an invocation
-// of method MaybeStartClone on an instance of MockService.
-type ServiceMaybeStartCloneFuncCall struct {
-	// Arg0 is the value of the 1st argument passed to this method
-	// invocation.
-	Arg0 context.Context
-	// Arg1 is the value of the 2nd argument passed to this method
-	// invocation.
-	Arg1 api.RepoName
-	// Result0 is the value of the 1st result returned from this method
-	// invocation.
-	Result0 bool
-	// Result1 is the value of the 2nd result returned from this method
-	// invocation.
-	Result1 CloneStatus
-	// Result2 is the value of the 3rd result returned from this method
-	// invocation.
-	Result2 error
-}
-
-// Args returns an interface slice containing the arguments of this
-// invocation.
-func (c ServiceMaybeStartCloneFuncCall) Args() []interface{} {
-	return []interface{}{c.Arg0, c.Arg1}
-}
-
-// Results returns an interface slice containing the results of this
-// invocation.
-func (c ServiceMaybeStartCloneFuncCall) Results() []interface{} {
-	return []interface{}{c.Result0, c.Result1, c.Result2}
-}
-
 // ServiceRepoUpdateFunc describes the behavior when the RepoUpdate method
 // of the parent MockService instance is invoked.
 type ServiceRepoUpdateFunc struct {

cmd/gitserver/internal/server_grpc.go

@@ -23,10 +23,12 @@ import (
 	"github.com/sourcegraph/sourcegraph/internal/authz"
 	"github.com/sourcegraph/sourcegraph/internal/conf"
 	"github.com/sourcegraph/sourcegraph/internal/database"
+	"github.com/sourcegraph/sourcegraph/internal/dotcom"
 	"github.com/sourcegraph/sourcegraph/internal/gitserver/gitdomain"
 	"github.com/sourcegraph/sourcegraph/internal/gitserver/protocol"
 	proto "github.com/sourcegraph/sourcegraph/internal/gitserver/v1"
 	"github.com/sourcegraph/sourcegraph/internal/grpc/streamio"
+	"github.com/sourcegraph/sourcegraph/internal/repoupdater"
 	"github.com/sourcegraph/sourcegraph/internal/trace"
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 	"github.com/sourcegraph/sourcegraph/lib/pointers"
@@ -35,7 +37,6 @@ import (
 type service interface {
 	CreateCommitFromPatch(ctx context.Context, req protocol.CreateCommitFromPatchRequest, patchReader io.Reader) protocol.CreateCommitFromPatchResponse
 	LogIfCorrupt(context.Context, api.RepoName, error)
-	MaybeStartClone(ctx context.Context, repo api.RepoName) (cloned bool, status CloneStatus, _ error)
 	IsRepoCloneable(ctx context.Context, repo api.RepoName) (protocol.IsRepoCloneableResponse, error)
 	RepoUpdate(ctx context.Context, req *protocol.RepoUpdateRequest) protocol.RepoUpdateResponse
 	SearchWithObservability(ctx context.Context, tr trace.Trace, args *protocol.SearchRequest, onMatch func(*protocol.CommitMatch) error) (limitHit bool, err error)
@@ -147,7 +148,7 @@ func (gs *grpcServer) Exec(req *proto.ExecRequest, ss proto.GitserverService_Exe
 	repoDir := gs.fs.RepoDir(repoName)
 	backend := gs.getBackendFunc(repoDir, repoName)
 
-	if err := gs.maybeStartClone(ctx, repoName); err != nil {
+	if err := gs.checkRepoExists(ctx, repoName); err != nil {
 		return err
 	}
 
@@ -265,7 +266,7 @@ func (gs *grpcServer) Archive(req *proto.ArchiveRequest, ss proto.GitserverServi
 	repoName := api.RepoName(req.GetRepo())
 	repoDir := gs.fs.RepoDir(repoName)
 
-	if err := gs.maybeStartClone(ss.Context(), repoName); err != nil {
+	if err := gs.checkRepoExists(ss.Context(), repoName); err != nil {
 		return err
 	}
 
@@ -390,7 +391,7 @@ func (gs *grpcServer) Search(req *proto.SearchRequest, ss proto.GitserverService
 
 	repoName := api.RepoName(req.GetRepo())
 
-	if err := gs.maybeStartClone(ss.Context(), repoName); err != nil {
+	if err := gs.checkRepoExists(ss.Context(), repoName); err != nil {
 		return err
 	}
 
@@ -779,7 +780,7 @@ func (gs *grpcServer) MergeBase(ctx context.Context, req *proto.MergeBaseRequest
 	repoName := api.RepoName(req.GetRepoName())
 	repoDir := gs.fs.RepoDir(repoName)
 
-	if err := gs.maybeStartClone(ctx, repoName); err != nil {
+	if err := gs.checkRepoExists(ctx, repoName); err != nil {
 		return nil, err
 	}
 
@@ -827,7 +828,7 @@ func (gs *grpcServer) GetCommit(ctx context.Context, req *proto.GetCommitRequest
 	repoName := api.RepoName(req.GetRepoName())
 	repoDir := gs.fs.RepoDir(repoName)
 
-	if err := gs.maybeStartClone(ctx, repoName); err != nil {
+	if err := gs.checkRepoExists(ctx, repoName); err != nil {
 		return nil, err
 	}
 
@@ -902,7 +903,7 @@ func (gs *grpcServer) Blame(req *proto.BlameRequest, ss proto.GitserverService_B
 	repoName := api.RepoName(req.GetRepoName())
 	repoDir := gs.fs.RepoDir(repoName)
 
-	if err := gs.maybeStartClone(ctx, repoName); err != nil {
+	if err := gs.checkRepoExists(ctx, repoName); err != nil {
 		return err
 	}
 
@@ -1001,7 +1002,7 @@ func (gs *grpcServer) DefaultBranch(ctx context.Context, req *proto.DefaultBranc
 	repoName := api.RepoName(req.GetRepoName())
 	repoDir := gs.fs.RepoDir(repoName)
 
-	if err := gs.maybeStartClone(ctx, repoName); err != nil {
+	if err := gs.checkRepoExists(ctx, repoName); err != nil {
 		return nil, err
 	}
 
@@ -1062,7 +1063,7 @@ func (gs *grpcServer) ReadFile(req *proto.ReadFileRequest, ss proto.GitserverSer
 	repoName := api.RepoName(req.GetRepoName())
 	repoDir := gs.fs.RepoDir(repoName)
 
-	if err := gs.maybeStartClone(ctx, repoName); err != nil {
+	if err := gs.checkRepoExists(ctx, repoName); err != nil {
 		return err
 	}
 
@@ -1141,7 +1142,7 @@ func (gs *grpcServer) ResolveRevision(ctx context.Context, req *proto.ResolveRev
 	repoName := api.RepoName(req.GetRepoName())
 	repoDir := gs.fs.RepoDir(repoName)
 
-	if err := gs.maybeStartClone(ctx, repoName); err != nil {
+	if err := gs.checkRepoExists(ctx, repoName); err != nil {
 		return nil, err
 	}
 
@@ -1185,17 +1186,37 @@ func (gs *grpcServer) ResolveRevision(ctx context.Context, req *proto.ResolveRev
 	}, nil
 }
 
-func (gs *grpcServer) maybeStartClone(ctx context.Context, repo api.RepoName) error {
-	cloned, state, err := gs.svc.MaybeStartClone(ctx, repo)
+// checkRepoExists checks if a given repository is cloned on disk, and returns an
+// error otherwise.
+// On Sourcegraph.com, not all repos are managed by the scheduler. We thus
+// need to enqueue a manual update of a repo that is visited but not cloned to
+// ensure it is cloned and managed.
+func (gs *grpcServer) checkRepoExists(ctx context.Context, repo api.RepoName) error {
+	cloned, err := gs.fs.RepoCloned(repo)
 	if err != nil {
-		return status.New(codes.Internal, "failed to check if repo is cloned").Err()
+		return status.New(codes.Internal, errors.Wrap(err, "failed to check if repo is cloned").Error()).Err()
 	}
 
 	if cloned {
 		return nil
 	}
 
-	return newRepoNotFoundError(repo, state.CloneInProgress, state.CloneProgress)
+	// On sourcegraph.com, not all repos are managed by the scheduler. We thus
+	// need to enqueue a manual clone of a repo that is visited but not cloned.
+	if dotcom.SourcegraphDotComMode() {
+		if conf.Get().DisableAutoGitUpdates {
+			gs.logger.Debug("not cloning on demand as DisableAutoGitUpdates is set")
+		} else {
+			_, err := repoupdater.DefaultClient.EnqueueRepoUpdate(ctx, repo)
+			if err != nil {
+				return errors.Wrap(err, "failed to enqueue repo clone")
+			}
+		}
+	}
+
+	cloneProgress, cloneInProgress := gs.locker.Status(repo)
+
+	return newRepoNotFoundError(repo, cloneInProgress, cloneProgress)
 }
 
 func hasAccessToCommit(ctx context.Context, repoName api.RepoName, files []string, checker authz.SubRepoPermissionChecker) (bool, error) {

cmd/gitserver/internal/server_grpc_test.go

@@ -57,25 +57,28 @@ func TestGRPCServer_Blame(t *testing.T) {
 		assertGRPCStatusCode(t, err, codes.InvalidArgument)
 	})
 	t.Run("checks for uncloned repo", func(t *testing.T) {
-		svc := NewMockService()
-		svc.MaybeStartCloneFunc.SetDefaultReturn(false, CloneStatus{CloneInProgress: true, CloneProgress: "cloning"}, nil)
-		gs := &grpcServer{svc: svc, fs: gitserverfs.NewMockFS()}
+		fs := gitserverfs.NewMockFS()
+		fs.RepoClonedFunc.SetDefaultReturn(false, nil)
+		locker := NewMockRepositoryLocker()
+		locker.StatusFunc.SetDefaultReturn("cloning", true)
+		gs := &grpcServer{svc: NewMockService(), fs: fs, locker: locker}
 		err := gs.Blame(&v1.BlameRequest{RepoName: "therepo", Commit: "deadbeef", Path: "thepath"}, mockSS)
 		require.Error(t, err)
 		assertGRPCStatusCode(t, err, codes.NotFound)
 		assertHasGRPCErrorDetailOfType(t, err, &proto.RepoNotFoundPayload{})
 		require.Contains(t, err.Error(), "repo not found")
-		mockassert.Called(t, svc.MaybeStartCloneFunc)
+		mockassert.Called(t, fs.RepoClonedFunc)
+		mockassert.Called(t, locker.StatusFunc)
 	})
 	t.Run("checks for subrepo perms access to given path", func(t *testing.T) {
 		srp := authz.NewMockSubRepoPermissionChecker()
-		svc := NewMockService()
+		fs := gitserverfs.NewMockFS()
 		// Repo is cloned, proceed!
-		svc.MaybeStartCloneFunc.SetDefaultReturn(true, CloneStatus{}, nil)
+		fs.RepoClonedFunc.SetDefaultReturn(true, nil)
 		gs := &grpcServer{
 			subRepoChecker: srp,
-			svc:            svc,
-			fs:             gitserverfs.NewMockFS(),
+			svc:            NewMockService(),
+			fs:             fs,
 			getBackendFunc: func(common.GitDir, api.RepoName) git.GitBackend {
 				b := git.NewMockGitBackend()
 				hr := git.NewMockBlameHunkReader()
@@ -115,9 +118,9 @@ func TestGRPCServer_Blame(t *testing.T) {
 		srp := authz.NewMockSubRepoPermissionChecker()
 		// Skip subrepo perms checks.
 		srp.EnabledFunc.SetDefaultReturn(false)
-		svc := NewMockService()
+		fs := gitserverfs.NewMockFS()
 		// Repo is cloned, proceed!
-		svc.MaybeStartCloneFunc.SetDefaultReturn(true, CloneStatus{}, nil)
+		fs.RepoClonedFunc.SetDefaultReturn(true, nil)
 		b := git.NewMockGitBackend()
 		hr := git.NewMockBlameHunkReader()
 		hr.ReadFunc.PushReturn(&gitdomain.Hunk{CommitID: "deadbeef"}, nil)
@@ -125,8 +128,8 @@ func TestGRPCServer_Blame(t *testing.T) {
 		b.BlameFunc.PushReturn(hr, nil)
 		gs := &grpcServer{
 			subRepoChecker: srp,
-			svc:            svc,
-			fs:             gitserverfs.NewMockFS(),
+			svc:            NewMockService(),
+			fs:             fs,
 			getBackendFunc: func(common.GitDir, api.RepoName) git.GitBackend {
 				return b
 			},
@@ -198,26 +201,29 @@ func TestGRPCServer_DefaultBranch(t *testing.T) {
 		assertGRPCStatusCode(t, err, codes.InvalidArgument)
 	})
 	t.Run("checks for uncloned repo", func(t *testing.T) {
-		svc := NewMockService()
-		svc.MaybeStartCloneFunc.SetDefaultReturn(false, CloneStatus{CloneInProgress: true, CloneProgress: "cloning"}, nil)
-		gs := &grpcServer{svc: svc, fs: gitserverfs.NewMockFS()}
+		fs := gitserverfs.NewMockFS()
+		fs.RepoClonedFunc.SetDefaultReturn(false, nil)
+		locker := NewMockRepositoryLocker()
+		locker.StatusFunc.SetDefaultReturn("cloning", true)
+		gs := &grpcServer{svc: NewMockService(), fs: fs, locker: locker}
 		_, err := gs.DefaultBranch(ctx, &v1.DefaultBranchRequest{RepoName: "therepo"})
 		require.Error(t, err)
 		assertGRPCStatusCode(t, err, codes.NotFound)
 		assertHasGRPCErrorDetailOfType(t, err, &proto.RepoNotFoundPayload{})
 		require.Contains(t, err.Error(), "repo not found")
-		mockassert.Called(t, svc.MaybeStartCloneFunc)
+		mockassert.Called(t, fs.RepoClonedFunc)
+		mockassert.Called(t, locker.StatusFunc)
 	})
 	t.Run("e2e", func(t *testing.T) {
-		svc := NewMockService()
+		fs := gitserverfs.NewMockFS()
 		// Repo is cloned, proceed!
-		svc.MaybeStartCloneFunc.SetDefaultReturn(true, CloneStatus{}, nil)
+		fs.RepoClonedFunc.SetDefaultReturn(true, nil)
 		b := git.NewMockGitBackend()
 		b.SymbolicRefHeadFunc.SetDefaultReturn("refs/heads/main", nil)
 		b.RevParseHeadFunc.SetDefaultReturn("deadbeef", nil)
 		gs := &grpcServer{
-			svc: svc,
-			fs:  gitserverfs.NewMockFS(),
+			svc: NewMockService(),
+			fs:  fs,
 			getBackendFunc: func(common.GitDir, api.RepoName) git.GitBackend {
 				return b
 			},
@@ -261,22 +267,26 @@ func TestGRPCServer_MergeBase(t *testing.T) {
 		assertGRPCStatusCode(t, err, codes.InvalidArgument)
 	})
 	t.Run("checks for uncloned repo", func(t *testing.T) {
-		svc := NewMockService()
-		svc.MaybeStartCloneFunc.SetDefaultReturn(false, CloneStatus{CloneInProgress: true, CloneProgress: "cloning"}, nil)
-		gs := &grpcServer{svc: svc, fs: gitserverfs.NewMockFS()}
+		fs := gitserverfs.NewMockFS()
+		fs.RepoClonedFunc.SetDefaultReturn(false, nil)
+		locker := NewMockRepositoryLocker()
+		locker.StatusFunc.SetDefaultReturn("cloning", true)
+		gs := &grpcServer{svc: NewMockService(), fs: fs, locker: locker}
 		_, err := gs.MergeBase(ctx, &v1.MergeBaseRequest{RepoName: "therepo", Base: []byte("master"), Head: []byte("b2")})
 		require.Error(t, err)
 		assertGRPCStatusCode(t, err, codes.NotFound)
 		assertHasGRPCErrorDetailOfType(t, err, &proto.RepoNotFoundPayload{})
 		require.Contains(t, err.Error(), "repo not found")
-		mockassert.Called(t, svc.MaybeStartCloneFunc)
+		mockassert.Called(t, fs.RepoClonedFunc)
+		mockassert.Called(t, locker.StatusFunc)
 	})
 	t.Run("revision not found", func(t *testing.T) {
-		svc := NewMockService()
-		svc.MaybeStartCloneFunc.SetDefaultReturn(true, CloneStatus{}, nil)
+		fs := gitserverfs.NewMockFS()
+		// Repo is cloned, proceed!
+		fs.RepoClonedFunc.SetDefaultReturn(true, nil)
 		gs := &grpcServer{
-			svc: svc,
-			fs:  gitserverfs.NewMockFS(),
+			svc: NewMockService(),
+			fs:  fs,
 			getBackendFunc: func(common.GitDir, api.RepoName) git.GitBackend {
 				b := git.NewMockGitBackend()
 				b.MergeBaseFunc.SetDefaultReturn("", &gitdomain.RevisionNotFoundError{Repo: "therepo", Spec: "b2"})
@@ -290,14 +300,14 @@ func TestGRPCServer_MergeBase(t *testing.T) {
 		require.Contains(t, err.Error(), "revision not found")
 	})
 	t.Run("e2e", func(t *testing.T) {
-		svc := NewMockService()
+		fs := gitserverfs.NewMockFS()
 		// Repo is cloned, proceed!
-		svc.MaybeStartCloneFunc.SetDefaultReturn(true, CloneStatus{}, nil)
+		fs.RepoClonedFunc.SetDefaultReturn(true, nil)
 		b := git.NewMockGitBackend()
 		b.MergeBaseFunc.SetDefaultReturn("deadbeef", nil)
 		gs := &grpcServer{
-			svc: svc,
-			fs:  gitserverfs.NewMockFS(),
+			svc: NewMockService(),
+			fs:  fs,
 			getBackendFunc: func(common.GitDir, api.RepoName) git.GitBackend {
 				return b
 			},
@@ -336,25 +346,28 @@ func TestGRPCServer_ReadFile(t *testing.T) {
 		assertGRPCStatusCode(t, err, codes.InvalidArgument)
 	})
 	t.Run("checks for uncloned repo", func(t *testing.T) {
-		svc := NewMockService()
-		svc.MaybeStartCloneFunc.SetDefaultReturn(false, CloneStatus{CloneInProgress: true, CloneProgress: "cloning"}, nil)
-		gs := &grpcServer{svc: svc, fs: gitserverfs.NewMockFS()}
+		fs := gitserverfs.NewMockFS()
+		fs.RepoClonedFunc.SetDefaultReturn(false, nil)
+		locker := NewMockRepositoryLocker()
+		locker.StatusFunc.SetDefaultReturn("cloning", true)
+		gs := &grpcServer{svc: NewMockService(), fs: fs, locker: locker}
 		err := gs.ReadFile(&v1.ReadFileRequest{RepoName: "therepo", Commit: "deadbeef", Path: "thepath"}, mockSS)
 		require.Error(t, err)
 		assertGRPCStatusCode(t, err, codes.NotFound)
 		assertHasGRPCErrorDetailOfType(t, err, &proto.RepoNotFoundPayload{})
 		require.Contains(t, err.Error(), "repo not found")
-		mockassert.Called(t, svc.MaybeStartCloneFunc)
+		mockassert.Called(t, fs.RepoClonedFunc)
+		mockassert.Called(t, locker.StatusFunc)
 	})
 	t.Run("checks for subrepo perms access to given path", func(t *testing.T) {
 		srp := authz.NewMockSubRepoPermissionChecker()
-		svc := NewMockService()
+		fs := gitserverfs.NewMockFS()
 		// Repo is cloned, proceed!
-		svc.MaybeStartCloneFunc.SetDefaultReturn(true, CloneStatus{}, nil)
+		fs.RepoClonedFunc.SetDefaultReturn(true, nil)
 		gs := &grpcServer{
 			subRepoChecker: srp,
-			svc:            svc,
-			fs:             gitserverfs.NewMockFS(),
+			svc:            NewMockService(),
+			fs:             fs,
 			getBackendFunc: func(common.GitDir, api.RepoName) git.GitBackend {
 				b := git.NewMockGitBackend()
 				b.ReadFileFunc.SetDefaultReturn(io.NopCloser(bytes.NewReader([]byte("filecontent"))), nil)
@@ -392,15 +405,15 @@ func TestGRPCServer_ReadFile(t *testing.T) {
 		srp := authz.NewMockSubRepoPermissionChecker()
 		// Skip subrepo perms checks.
 		srp.EnabledFunc.SetDefaultReturn(false)
-		svc := NewMockService()
+		fs := gitserverfs.NewMockFS()
 		// Repo is cloned, proceed!
-		svc.MaybeStartCloneFunc.SetDefaultReturn(true, CloneStatus{}, nil)
+		fs.RepoClonedFunc.SetDefaultReturn(true, nil)
 		b := git.NewMockGitBackend()
 		b.ReadFileFunc.SetDefaultReturn(io.NopCloser(bytes.NewReader([]byte("filecontent"))), nil)
 		gs := &grpcServer{
 			subRepoChecker: srp,
-			svc:            svc,
-			fs:             gitserverfs.NewMockFS(),
+			svc:            NewMockService(),
+			fs:             fs,
 			getBackendFunc: func(common.GitDir, api.RepoName) git.GitBackend {
 				return b
 			},
@@ -474,25 +487,28 @@ func TestGRPCServer_Archive(t *testing.T) {
 		assertGRPCStatusCode(t, err, codes.InvalidArgument)
 	})
 	t.Run("checks for uncloned repo", func(t *testing.T) {
-		svc := NewMockService()
-		svc.MaybeStartCloneFunc.SetDefaultReturn(false, CloneStatus{CloneInProgress: true, CloneProgress: "cloning"}, nil)
-		gs := &grpcServer{svc: svc, fs: gitserverfs.NewMockFS()}
+		fs := gitserverfs.NewMockFS()
+		fs.RepoClonedFunc.SetDefaultReturn(false, nil)
+		locker := NewMockRepositoryLocker()
+		locker.StatusFunc.SetDefaultReturn("cloning", true)
+		gs := &grpcServer{svc: NewMockService(), fs: fs, locker: locker}
 		err := gs.Archive(&v1.ArchiveRequest{Repo: "therepo", Treeish: "HEAD", Format: proto.ArchiveFormat_ARCHIVE_FORMAT_ZIP}, mockSS)
 		require.Error(t, err)
 		assertGRPCStatusCode(t, err, codes.NotFound)
 		assertHasGRPCErrorDetailOfType(t, err, &proto.RepoNotFoundPayload{})
 		require.Contains(t, err.Error(), "repo not found")
-		mockassert.Called(t, svc.MaybeStartCloneFunc)
+		mockassert.Called(t, fs.RepoClonedFunc)
+		mockassert.Called(t, locker.StatusFunc)
 	})
 	t.Run("checks if sub-repo perms are enabled for repo", func(t *testing.T) {
 		srp := authz.NewMockSubRepoPermissionChecker()
-		svc := NewMockService()
+		fs := gitserverfs.NewMockFS()
 		// Repo is cloned, proceed!
-		svc.MaybeStartCloneFunc.SetDefaultReturn(true, CloneStatus{}, nil)
+		fs.RepoClonedFunc.SetDefaultReturn(true, nil)
 		gs := &grpcServer{
 			subRepoChecker: srp,
-			svc:            svc,
-			fs:             gitserverfs.NewMockFS(),
+			svc:            NewMockService(),
+			fs:             fs,
 			getBackendFunc: func(common.GitDir, api.RepoName) git.GitBackend {
 				b := git.NewMockGitBackend()
 				b.ArchiveReaderFunc.SetDefaultReturn(io.NopCloser(bytes.NewReader([]byte("filecontent"))), nil)
@@ -530,15 +546,15 @@ func TestGRPCServer_Archive(t *testing.T) {
 		srp := authz.NewMockSubRepoPermissionChecker()
 		// Skip subrepo perms checks.
 		srp.EnabledForRepoFunc.SetDefaultReturn(false, nil)
-		svc := NewMockService()
+		fs := gitserverfs.NewMockFS()
 		// Repo is cloned, proceed!
-		svc.MaybeStartCloneFunc.SetDefaultReturn(true, CloneStatus{}, nil)
+		fs.RepoClonedFunc.SetDefaultReturn(true, nil)
 		b := git.NewMockGitBackend()
 		b.ArchiveReaderFunc.SetDefaultReturn(io.NopCloser(bytes.NewReader([]byte("filecontent"))), nil)
 		gs := &grpcServer{
 			subRepoChecker: srp,
-			svc:            svc,
-			fs:             gitserverfs.NewMockFS(),
+			svc:            NewMockService(),
+			fs:             fs,
 			getBackendFunc: func(common.GitDir, api.RepoName) git.GitBackend {
 				return b
 			},
@@ -608,26 +624,29 @@ func TestGRPCServer_GetCommit(t *testing.T) {
 		assertGRPCStatusCode(t, err, codes.InvalidArgument)
 	})
 	t.Run("checks for uncloned repo", func(t *testing.T) {
-		svc := NewMockService()
-		svc.MaybeStartCloneFunc.SetDefaultReturn(false, CloneStatus{CloneInProgress: true, CloneProgress: "cloning"}, nil)
-		gs := &grpcServer{svc: svc, fs: gitserverfs.NewMockFS()}
+		fs := gitserverfs.NewMockFS()
+		fs.RepoClonedFunc.SetDefaultReturn(false, nil)
+		locker := NewMockRepositoryLocker()
+		locker.StatusFunc.SetDefaultReturn("cloning", true)
+		gs := &grpcServer{svc: NewMockService(), fs: fs, locker: locker}
 		_, err := gs.GetCommit(ctx, &v1.GetCommitRequest{RepoName: "therepo", Commit: "deadbeef"})
 		require.Error(t, err)
 		assertGRPCStatusCode(t, err, codes.NotFound)
 		assertHasGRPCErrorDetailOfType(t, err, &proto.RepoNotFoundPayload{})
 		require.Contains(t, err.Error(), "repo not found")
-		mockassert.Called(t, svc.MaybeStartCloneFunc)
+		mockassert.Called(t, fs.RepoClonedFunc)
+		mockassert.Called(t, locker.StatusFunc)
 	})
 	t.Run("checks for subrepo perms access to commit", func(t *testing.T) {
 		srp := authz.NewMockSubRepoPermissionChecker()
-		svc := NewMockService()
+		fs := gitserverfs.NewMockFS()
 		// Repo is cloned, proceed!
-		svc.MaybeStartCloneFunc.SetDefaultReturn(true, CloneStatus{}, nil)
+		fs.RepoClonedFunc.SetDefaultReturn(true, nil)
 		b := git.NewMockGitBackend()
 		gs := &grpcServer{
 			subRepoChecker: srp,
-			svc:            svc,
-			fs:             gitserverfs.NewMockFS(),
+			svc:            NewMockService(),
+			fs:             fs,
 			getBackendFunc: func(common.GitDir, api.RepoName) git.GitBackend {
 				return b
 			},
@@ -681,15 +700,15 @@ func TestGRPCServer_GetCommit(t *testing.T) {
 		// Skip subrepo perms checks.
 		srp.EnabledFunc.SetDefaultReturn(false)
 		srp.EnabledForRepoFunc.SetDefaultReturn(false, nil)
-		svc := NewMockService()
+		fs := gitserverfs.NewMockFS()
 		// Repo is cloned, proceed!
-		svc.MaybeStartCloneFunc.SetDefaultReturn(true, CloneStatus{}, nil)
+		fs.RepoClonedFunc.SetDefaultReturn(true, nil)
 		b := git.NewMockGitBackend()
 		b.GetCommitFunc.PushReturn(&git.GitCommitWithFiles{Commit: &gitdomain.Commit{Committer: &gitdomain.Signature{}}}, nil)
 		gs := &grpcServer{
 			subRepoChecker: srp,
-			svc:            svc,
-			fs:             gitserverfs.NewMockFS(),
+			svc:            NewMockService(),
+			fs:             fs,
 			getBackendFunc: func(common.GitDir, api.RepoName) git.GitBackend {
 				return b
 			},
@@ -722,25 +741,29 @@ func TestGRPCServer_ResolveRevision(t *testing.T) {
 		assertGRPCStatusCode(t, err, codes.InvalidArgument)
 	})
 	t.Run("checks for uncloned repo", func(t *testing.T) {
-		svc := NewMockService()
-		svc.MaybeStartCloneFunc.SetDefaultReturn(false, CloneStatus{CloneInProgress: true, CloneProgress: "cloning"}, nil)
-		gs := &grpcServer{svc: svc, fs: gitserverfs.NewMockFS()}
+		fs := gitserverfs.NewMockFS()
+		fs.RepoClonedFunc.SetDefaultReturn(false, nil)
+		locker := NewMockRepositoryLocker()
+		locker.StatusFunc.SetDefaultReturn("cloning", true)
+		gs := &grpcServer{svc: NewMockService(), fs: fs, locker: locker}
 		_, err := gs.ResolveRevision(ctx, &v1.ResolveRevisionRequest{RepoName: "therepo"})
 		require.Error(t, err)
 		assertGRPCStatusCode(t, err, codes.NotFound)
 		assertHasGRPCErrorDetailOfType(t, err, &proto.RepoNotFoundPayload{})
 		require.Contains(t, err.Error(), "repo not found")
-		mockassert.Called(t, svc.MaybeStartCloneFunc)
+		mockassert.Called(t, fs.RepoClonedFunc)
+		mockassert.Called(t, locker.StatusFunc)
 	})
 	t.Run("e2e", func(t *testing.T) {
-		svc := NewMockService()
+		fs := gitserverfs.NewMockFS()
 		// Repo is cloned, proceed!
-		svc.MaybeStartCloneFunc.SetDefaultReturn(true, CloneStatus{}, nil)
+		fs.RepoClonedFunc.SetDefaultReturn(true, nil)
 		b := git.NewMockGitBackend()
 		b.ResolveRevisionFunc.SetDefaultReturn("deadbeef", nil)
+		svc := NewMockService()
 		gs := &grpcServer{
 			svc: svc,
-			fs:  gitserverfs.NewMockFS(),
+			fs:  fs,
 			getBackendFunc: func(common.GitDir, api.RepoName) git.GitBackend {
 				return b
 			},

cmd/gitserver/internal/server_test.go

@@ -115,7 +115,7 @@ func TestExecRequest(t *testing.T) {
 	db := dbmocks.NewMockDB()
 	gr := dbmocks.NewMockGitserverRepoStore()
 	db.GitserverReposFunc.SetDefaultReturn(gr)
-	fs := gitserverfs.New(observation.TestContextTB(t), t.TempDir())
+	fs := gitserverfs.NewMockFSFrom(gitserverfs.New(&observation.TestContext, t.TempDir()))
 	require.NoError(t, fs.Initialize())
 	s := NewServer(&ServerOpts{
 		Logger: logtest.Scoped(t),
@@ -177,15 +177,14 @@ func TestExecRequest(t *testing.T) {
 	})
 
 	gs := NewGRPCServer(s)
-	svc := NewMockServiceFrom(s)
-	svc.MaybeStartCloneFunc.SetDefaultHook(func(ctx context.Context, repo api.RepoName) (bool, CloneStatus, error) {
+	fs.RepoClonedFunc.SetDefaultHook(func(repo api.RepoName) (bool, error) {
 		if repo == "github.com/gorilla/mux" || repo == "my-mux" {
-			return true, CloneStatus{}, nil
+			return true, nil
 		}
-		cloneProgress, err := s.CloneRepo(ctx, repo, CloneOptions{})
-		return false, CloneStatus{CloneProgress: cloneProgress, CloneInProgress: err != nil}, nil
+		_, err := s.CloneRepo(context.Background(), repo, CloneOptions{})
+		require.NoError(t, err)
+		return false, nil
 	})
-	gs.(*grpcServer).svc = svc
 
 	vcssyncer.TestGitRepoExists = func(ctx context.Context, repoName api.RepoName) error {
 		if strings.Contains(string(repoName), "nicksnyder/go-i18n") {

mockgen.test.yaml

@@ -421,3 +421,5 @@
     - path: github.com/sourcegraph/sourcegraph/cmd/gitserver/internal
       interfaces:
         - service
+        - RepositoryLocker
+        - RepositoryLock