repository: Detect locking issues in WriteRef

When WriteRef fails because it cannot lock the reference that is to be
updated it returns an internal error. Given that this is quite a likely
error scenario though and that we already have the infrastructure in
place to detect such issues we can do better though and return a more
telling error.

Refactor the RPC to instead return an explicit error with metadata. This
should make it easier for us to detect lock contention better going
forward.

2 files changed, 45 insertions, 2 deletions

diff --git a/internal/gitaly/service/repository/write_ref.go b/internal/gitaly/service/repository/write_ref.go
index 76c6d4190..3d7bec413 100644
--- a/internal/gitaly/service/repository/write_ref.go
+++ b/internal/gitaly/service/repository/write_ref.go
@@ -3,6 +3,7 @@ package repository
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
@@ -90,11 +91,16 @@ func updateRef(ctx context.Context, repo *localrepo.Repo, req *gitalypb.WriteRef
 		return fmt.Errorf("start reference transaction: %w", err)
 	}
 
-	if err = u.Update(git.ReferenceName(req.GetRef()), newObjectID, oldObjectID); err != nil {
+	if err := u.Update(git.ReferenceName(req.GetRef()), newObjectID, oldObjectID); err != nil {
 		return fmt.Errorf("error when creating update-ref command: %w", err)
 	}
 
-	if err = u.Commit(); err != nil {
+	if err := u.Commit(); err != nil {
+		var alreadyLockedErr updateref.AlreadyLockedError
+		if errors.As(err, &alreadyLockedErr) {
+			return structerr.NewAborted("reference is locked already").WithMetadata("reference", alreadyLockedErr.ReferenceName)
+		}
+
 		return fmt.Errorf("error when running update-ref command: %w", err)
 	}
 
diff --git a/internal/gitaly/service/repository/write_ref_test.go b/internal/gitaly/service/repository/write_ref_test.go
index b9f029c24..3cc700afb 100644
--- a/internal/gitaly/service/repository/write_ref_test.go
+++ b/internal/gitaly/service/repository/write_ref_test.go
@@ -9,6 +9,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/metadata"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
@@ -371,3 +372,39 @@ func TestWriteRef(t *testing.T) {
 		})
 	}
 }
+
+func TestWriteRef_locked(t *testing.T) {
+	t.Parallel()
+
+	ctx := testhelper.Context(t)
+	cfg, client := setupRepositoryServiceWithoutRepo(t)
+
+	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
+	repo := localrepo.NewTestRepo(t, cfg, repoProto)
+	commitID := gittest.WriteCommit(t, cfg, repoPath)
+
+	// Lock the reference that we're about to update via the RPC call.
+	updater, err := updateref.New(ctx, repo)
+	require.NoError(t, err)
+	defer testhelper.MustClose(t, updater)
+	require.NoError(t, updater.Start())
+	require.NoError(t, updater.Update(
+		git.ReferenceName("refs/heads/locked-branch"),
+		commitID,
+		gittest.DefaultObjectHash.ZeroOID,
+	))
+	require.NoError(t, updater.Prepare())
+
+	_, err = client.WriteRef(ctx, &gitalypb.WriteRefRequest{
+		Repository: repoProto,
+		Ref:        []byte("refs/heads/locked-branch"),
+		Revision:   []byte(commitID),
+	})
+	testhelper.RequireGrpcError(t,
+		testhelper.WithInterceptedMetadata(
+			structerr.NewAborted("reference is locked already"),
+			"reference", "refs/heads/locked-branch",
+		),
+		err,
+	)
+}