From a2b170dc9e68bfba670bfb235e3a99f9dd8216a6 Mon Sep 17 00:00:00 2001 From: Erik Seliger Date: Fri, 19 Apr 2024 00:00:49 +0200 Subject: [PATCH] gitserver: Clarify what RepositoryLock indicates (#61862) After refactoring to use the same locker for both tasks, this no longer only resembles cloneInProgress, so cleaned this up a little. Test plan: Existing test suites. --- cmd/gitserver/internal/repo_info.go | 6 +++++- cmd/gitserver/internal/server_grpc.go | 6 +++++- cmd/gitserver/internal/serverutil.go | 4 ++-- cmd/gitserver/internal/statesyncer.go | 4 ++-- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/cmd/gitserver/internal/repo_info.go b/cmd/gitserver/internal/repo_info.go index a9c5b5d7c4d..aacbf0eb4d3 100644 --- a/cmd/gitserver/internal/repo_info.go +++ b/cmd/gitserver/internal/repo_info.go @@ -20,11 +20,15 @@ func repoCloneProgress(fs gitserverfs.FS, locker RepositoryLocker, repo api.Repo resp := protocol.RepoCloneProgress{ Cloned: cloned, } - resp.CloneProgress, resp.CloneInProgress = locker.Status(repo) + cloneProgress, locked := locker.Status(repo) if isAlwaysCloningTest(repo) { resp.CloneInProgress = true resp.CloneProgress = "This will never finish cloning" } + if !cloned && locked { + resp.CloneInProgress = true + resp.CloneProgress = cloneProgress + } return &resp, nil } diff --git a/cmd/gitserver/internal/server_grpc.go b/cmd/gitserver/internal/server_grpc.go index fb9540d44ea..08976e8bd9f 100644 --- a/cmd/gitserver/internal/server_grpc.go +++ b/cmd/gitserver/internal/server_grpc.go @@ -1422,7 +1422,11 @@ func (gs *grpcServer) checkRepoExists(ctx context.Context, repo api.RepoName) er } } - cloneProgress, cloneInProgress := gs.locker.Status(repo) + cloneProgress, locked := gs.locker.Status(repo) + + // We checked above that the repo is not cloned. So if the repo is currently + // locked, it must be a clone in progress. + cloneInProgress := locked return newRepoNotFoundError(repo, cloneInProgress, cloneProgress) } diff --git a/cmd/gitserver/internal/serverutil.go b/cmd/gitserver/internal/serverutil.go index a55695b0e05..6843b298c6c 100644 --- a/cmd/gitserver/internal/serverutil.go +++ b/cmd/gitserver/internal/serverutil.go @@ -14,11 +14,11 @@ import ( "github.com/sourcegraph/sourcegraph/internal/types" ) -func cloneStatus(cloned, cloning bool) types.CloneStatus { +func cloneStatus(cloned, locked bool) types.CloneStatus { switch { case cloned: return types.CloneStatusCloned - case cloning: + case locked: return types.CloneStatusCloning } return types.CloneStatusNotCloned diff --git a/cmd/gitserver/internal/statesyncer.go b/cmd/gitserver/internal/statesyncer.go index f6da8b413d8..05bed03485e 100644 --- a/cmd/gitserver/internal/statesyncer.go +++ b/cmd/gitserver/internal/statesyncer.go @@ -201,14 +201,14 @@ func syncRepoState( // Failed to determine cloned state, we have to skip this record for now. continue } - _, cloning := locker.Status(repo.Name) + _, locked := locker.Status(repo.Name) var shouldUpdate bool if repo.ShardID != shardID { repo.ShardID = shardID shouldUpdate = true } - cloneStatus := cloneStatus(cloned, cloning) + cloneStatus := cloneStatus(cloned, locked) if repo.CloneStatus != cloneStatus { repo.CloneStatus = cloneStatus // Since the repo has been recloned or is being cloned