Refactor Snapshot as SnapshotLSN

The Snapshot type contained previously more fields. With the introduction
of the file system level snapshots, we no longer had to track for example
the version of the hooks that should be used in the transaction. Nowadays
the Snapshot only contains the read index field, which is the log index the
transaction is reading the partition's state at. Simplify by removing the
Snapshot type and replacing it with snapshotLSN field that directly records
the LSN the transaction is reading at.

2 files changed, 111 insertions, 214 deletions

diff --git a/internal/gitaly/storage/storagemgr/transaction_manager.go b/internal/gitaly/storage/storagemgr/transaction_manager.go
index df0b7909c..9283fa580 100644
--- a/internal/gitaly/storage/storagemgr/transaction_manager.go
+++ b/internal/gitaly/storage/storagemgr/transaction_manager.go
@@ -114,12 +114,6 @@ type CustomHooksUpdate struct {
 // is the expected old tip and the desired new tip.
 type ReferenceUpdates map[git.ReferenceName]ReferenceUpdate
 
-// Snapshot contains the read snapshot details of a Transaction.
-type Snapshot struct {
-	// ReadIndex is the index of the log entry this Transaction is reading the data at.
-	ReadIndex LogIndex
-}
-
 type transactionState int
 
 const (
@@ -174,8 +168,9 @@ type Transaction struct {
 	// if this is a read-write transaction.
 	snapshotRepository *localrepo.Repo
 
-	// Snapshot contains the details of the Transaction's read snapshot.
-	snapshot Snapshot
+	// snapshotLSN is the log sequence number which this transaction is reading the repository's
+	// state at.
+	snapshotLSN LogIndex
 
 	skipVerificationFailures bool
 	initialReferenceValues   map[git.ReferenceName]git.ObjectID
@@ -215,14 +210,14 @@ func (mgr *TransactionManager) Begin(ctx context.Context, opts TransactionOption
 	txn := &Transaction{
 		readOnly:     opts.ReadOnly,
 		commit:       mgr.commit,
-		snapshot:     Snapshot{ReadIndex: mgr.appendedLogIndex},
+		snapshotLSN:  mgr.appendedLogIndex,
 		finished:     make(chan struct{}),
 		relativePath: mgr.relativePath,
 	}
 
-	mgr.snapshotLocks[txn.snapshot.ReadIndex].activeSnapshotters.Add(1)
-	defer mgr.snapshotLocks[txn.snapshot.ReadIndex].activeSnapshotters.Done()
-	readReady := mgr.snapshotLocks[txn.snapshot.ReadIndex].applied
+	mgr.snapshotLocks[txn.snapshotLSN].activeSnapshotters.Add(1)
+	defer mgr.snapshotLocks[txn.snapshotLSN].activeSnapshotters.Done()
+	readReady := mgr.snapshotLocks[txn.snapshotLSN].applied
 	mgr.mutex.Unlock()
 
 	txn.finish = func() error {
@@ -460,9 +455,9 @@ func (txn *Transaction) finishUnadmitted() error {
 	return txn.finish()
 }
 
-// Snapshot returns the details of the Transaction's read snapshot.
-func (txn *Transaction) Snapshot() Snapshot {
-	return txn.snapshot
+// SnapshotLSN returns the LSN of the Transaction's read snapshot.
+func (txn *Transaction) SnapshotLSN() LogIndex {
+	return txn.snapshotLSN
 }
 
 // SkipVerificationFailures configures the transaction to skip reference updates that fail verification.
diff --git a/internal/gitaly/storage/storagemgr/transaction_manager_test.go b/internal/gitaly/storage/storagemgr/transaction_manager_test.go
index 85ac09a8b..0add472af 100644
--- a/internal/gitaly/storage/storagemgr/transaction_manager_test.go
+++ b/internal/gitaly/storage/storagemgr/transaction_manager_test.go
@@ -315,8 +315,8 @@ func TestTransactionManager(t *testing.T) {
 		TransactionOptions TransactionOptions
 		// Context is the context to use for the Begin call.
 		Context context.Context
-		// ExpectedSnapshot is the expected snapshot of the transaction.
-		ExpectedSnapshot Snapshot
+		// ExpectedSnapshot is the expected LSN this transaction should read the repsoitory's state at.
+		ExpectedSnapshotLSN LogIndex
 		// ExpectedError is the error expected to be returned from the Begin call.
 		ExpectedError error
 	}
@@ -544,10 +544,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -591,10 +589,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -623,10 +619,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -700,10 +694,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -815,10 +807,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID:            2,
@@ -858,10 +848,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -902,10 +890,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -945,10 +931,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -984,10 +968,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID:            2,
@@ -1028,10 +1010,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -1092,10 +1072,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -1130,10 +1108,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -1193,10 +1169,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -1239,10 +1213,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -1282,10 +1254,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID:            2,
@@ -1323,10 +1293,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -1505,10 +1473,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				RecordInitialReferenceValues{
 					TransactionID: 2,
@@ -1552,10 +1518,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				RecordInitialReferenceValues{
 					TransactionID: 2,
@@ -1599,10 +1563,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				RecordInitialReferenceValues{
 					TransactionID: 2,
@@ -1716,10 +1678,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID:     2,
@@ -1836,28 +1796,22 @@ func TestTransactionManager(t *testing.T) {
 				},
 				StartManager{},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID:     2,
 					CustomHooksUpdate: &CustomHooksUpdate{},
 				},
 				Begin{
-					TransactionID: 3,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 2,
-					},
+					TransactionID:       3,
+					ExpectedSnapshotLSN: 2,
 				},
 				CloseManager{},
 				StartManager{},
 				Begin{
-					TransactionID: 4,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 2,
-					},
+					TransactionID:       4,
+					ExpectedSnapshotLSN: 2,
 				},
 				Rollback{
 					TransactionID: 4,
@@ -1937,10 +1891,8 @@ func TestTransactionManager(t *testing.T) {
 				CloseManager{},
 				StartManager{},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -2029,10 +1981,8 @@ func TestTransactionManager(t *testing.T) {
 				},
 				StartManager{},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -2076,10 +2026,8 @@ func TestTransactionManager(t *testing.T) {
 				AssertManager{},
 				StartManager{},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -2123,10 +2071,8 @@ func TestTransactionManager(t *testing.T) {
 				AssertManager{},
 				StartManager{},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -2281,10 +2227,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -2322,10 +2266,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -2425,10 +2367,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -2469,10 +2409,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -2529,10 +2467,8 @@ func TestTransactionManager(t *testing.T) {
 				},
 				StartManager{},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -2623,10 +2559,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 3,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       3,
+					ExpectedSnapshotLSN: 1,
 				},
 				// Transaction 3 is should see the new changes as it began after transaction 1 was committed.
 				RepositoryAssertion{
@@ -2663,19 +2597,15 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 4,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 2,
-					},
+					TransactionID:       4,
+					ExpectedSnapshotLSN: 2,
 				},
 				Rollback{
 					TransactionID: 3,
 				},
 				Begin{
-					TransactionID: 5,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 2,
-					},
+					TransactionID:       5,
+					ExpectedSnapshotLSN: 2,
 				},
 				Commit{
 					TransactionID: 4,
@@ -2685,10 +2615,8 @@ func TestTransactionManager(t *testing.T) {
 					CustomHooksUpdate: &CustomHooksUpdate{},
 				},
 				Begin{
-					TransactionID: 6,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 3,
-					},
+					TransactionID:       6,
+					ExpectedSnapshotLSN: 3,
 				},
 				Rollback{
 					TransactionID: 5,
@@ -2798,10 +2726,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				// Point main to the first commit so the second one is unreachable.
 				Commit{
@@ -2823,10 +2749,8 @@ func TestTransactionManager(t *testing.T) {
 					ExpectedError: errSimulatedCrash,
 				},
 				Begin{
-					TransactionID: 3,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 2,
-					},
+					TransactionID:       3,
+					ExpectedSnapshotLSN: 2,
 				},
 				Commit{
 					TransactionID: 3,
@@ -2995,10 +2919,8 @@ func TestTransactionManager(t *testing.T) {
 					QuarantinedPacks: [][]byte{setup.Commits.First.Pack},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -3202,10 +3124,8 @@ func TestTransactionManager(t *testing.T) {
 					QuarantinedPacks: [][]byte{setup.Commits.First.Pack},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -3259,16 +3179,12 @@ func TestTransactionManager(t *testing.T) {
 					QuarantinedPacks: [][]byte{setup.Commits.First.Pack},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Begin{
-					TransactionID: 3,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       3,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID: 2,
@@ -3352,10 +3268,8 @@ func TestTransactionManager(t *testing.T) {
 					DeleteRepository: true,
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				RepositoryAssertion{
 					TransactionID: 2,
@@ -3508,10 +3422,8 @@ func TestTransactionManager(t *testing.T) {
 				},
 				StartManager{},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				RepositoryAssertion{
 					TransactionID: 2,
@@ -3552,10 +3464,8 @@ func TestTransactionManager(t *testing.T) {
 				},
 				StartManager{},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				RepositoryAssertion{
 					TransactionID: 2,
@@ -3873,10 +3783,8 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 3,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       3,
+					ExpectedSnapshotLSN: 1,
 				},
 				// This transaction was started before the commit, so it should see the original state.
 				RepositoryAssertion{
@@ -3980,16 +3888,12 @@ func TestTransactionManager(t *testing.T) {
 					},
 				},
 				Begin{
-					TransactionID: 2,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       2,
+					ExpectedSnapshotLSN: 1,
 				},
 				Begin{
-					TransactionID: 3,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 1,
-					},
+					TransactionID:       3,
+					ExpectedSnapshotLSN: 1,
 				},
 				Commit{
 					TransactionID:    2,
@@ -4025,10 +3929,8 @@ func TestTransactionManager(t *testing.T) {
 					TransactionID: 3,
 				},
 				Begin{
-					TransactionID: 4,
-					ExpectedSnapshot: Snapshot{
-						ReadIndex: 2,
-					},
+					TransactionID:       4,
+					ExpectedSnapshotLSN: 2,
 				},
 				RepositoryAssertion{
 					TransactionID: 4,
@@ -4264,7 +4166,7 @@ func TestTransactionManager(t *testing.T) {
 					transaction, err := transactionManager.Begin(beginCtx, step.TransactionOptions)
 					require.Equal(t, step.ExpectedError, err)
 					if err == nil {
-						require.Equal(t, step.ExpectedSnapshot, transaction.Snapshot())
+						require.Equal(t, step.ExpectedSnapshotLSN, transaction.SnapshotLSN())
 					}
 
 					if step.TransactionOptions.ReadOnly {