Verify references against a snapshot

TransactionManager is currently verifying references against the
main repository. The verification process prepares a reference
transaction and considers the verification successful if no errors
come up. Preparing a reference transaction in the repository may
leave locks if there is a crash while the locks are held. As the
verification happens prior to logging the write, there's no record
of the fact that locks may have been left in the repository. As it
is, such locks would be left in the repository, and never cleaned
up. This commit runs verifies the references against a snapshot of
the target repository. This has few benefits:

1. The locks target the snapshot. If there is a crash while the locks
   are held, they'll be removed along with the snapshot when Gitaly
   restarts.
2. As the verification process no longer modifies the repository,
   transactions can take their snapshots concurrently with the
   reference verification running.
3. This paves the way for verifying more complex transactions by using
   multiple reference transactions. This is useful for example when a
   transaction resolves a directory-file conflict. Such updates can't
   be done in a single reference transaction.

In future we can also verify transactions and apply log to the repository
concurrently as the verification process no longer writes the locks to
the repository. This will especially improve throughput with Raft as
log entries don't have to be committed one by one anymore.

1 files changed, 17 insertions, 16 deletions

diff --git a/internal/gitaly/storage/storagemgr/transaction_manager.go b/internal/gitaly/storage/storagemgr/transaction_manager.go
index 78ba2fd5b..3a81848c8 100644
--- a/internal/gitaly/storage/storagemgr/transaction_manager.go
+++ b/internal/gitaly/storage/storagemgr/transaction_manager.go
@@ -293,8 +293,6 @@ func (mgr *TransactionManager) Begin(ctx context.Context, relativePath string, s
 			return nil, fmt.Errorf("mkdir temp: %w", err)
 		}
 
-		mgr.stateLock.RLock()
-		defer mgr.stateLock.RUnlock()
 		if txn.snapshot, err = newSnapshot(ctx,
 			mgr.storagePath,
 			filepath.Join(txn.stagingDirectory, "snapshot"),
@@ -658,9 +656,6 @@ type TransactionManager struct {
 	// Run and Begin which are ran in different goroutines.
 	mutex sync.Mutex
 
-	// stateLock is used to synchronize snapshotting with reference verification as the verification
-	// process targets the main repository and creates locks in it.
-	stateLock sync.RWMutex
 	// snapshotLocks contains state used for synchronizing snapshotters with the log application.
 	snapshotLocks map[LSN]*snapshotLock
 
@@ -1521,7 +1516,7 @@ func (mgr *TransactionManager) verifyReferences(ctx context.Context, transaction
 		) == -1
 	})
 
-	if err := mgr.verifyReferencesWithGit(ctx, referenceUpdates, transaction.stagingRepository); err != nil {
+	if err := mgr.verifyReferencesWithGit(ctx, referenceUpdates, transaction); err != nil {
 		return nil, fmt.Errorf("verify references with git: %w", err)
 	}
 
@@ -1532,17 +1527,23 @@ func (mgr *TransactionManager) verifyReferences(ctx context.Context, transaction
 // the updates will go through when they are being applied in the log. This also catches any invalid reference names
 // and file/directory conflicts with Git's loose reference storage which can occur with references like
 // 'refs/heads/parent' and 'refs/heads/parent/child'.
-func (mgr *TransactionManager) verifyReferencesWithGit(ctx context.Context, referenceUpdates []*gitalypb.LogEntry_ReferenceUpdate, quarantinedRepo *localrepo.Repo) error {
-	// Prevent snapshots from being taken concurrently as the reference verification
-	// currently creates locks in the repository. We don't need to synchronnize snapshotting
-	// and reference verification once we are no longer verifying the references against
-	// the repository.
-	//
-	// Alternatively this could be removed by ignoring the lock files during snapshot creation.
-	mgr.stateLock.Lock()
-	defer mgr.stateLock.Unlock()
+func (mgr *TransactionManager) verifyReferencesWithGit(ctx context.Context, referenceUpdates []*gitalypb.LogEntry_ReferenceUpdate, tx *Transaction) error {
+	relativePath := tx.stagingRepository.GetRelativePath()
+	snapshot, err := newSnapshot(ctx,
+		mgr.storagePath,
+		filepath.Join(tx.stagingDirectory, "staging"),
+		[]string{relativePath},
+	)
+	if err != nil {
+		return fmt.Errorf("new snapshot: %w", err)
+	}
+
+	repo, err := mgr.repositoryFactory.Build(snapshot.relativePath(relativePath)).Quarantine(tx.quarantineDirectory)
+	if err != nil {
+		return fmt.Errorf("quarantine: %w", err)
+	}
 
-	updater, err := mgr.prepareReferenceTransaction(ctx, referenceUpdates, quarantinedRepo)
+	updater, err := mgr.prepareReferenceTransaction(ctx, referenceUpdates, repo)
 	if err != nil {
 		return fmt.Errorf("prepare reference transaction: %w", err)
 	}