coordinator: Categorize RPCs by whether they need transaction or not

The list of repository-scoped mutating RPCs which is consulted for
whether a given RPC shall use transactions or not currently only has two
blocks, one for unconditionally enabled respectively disabled ones. In
the next commit, we're going to conditionally enable a subset of the
disabled RPCs via feature flags.

But in fact it doesn't make any sense to set up transactions for some of
the RPCs. If we know that a given RPC won't ever update any reference,
then it's fine to not use them. Most importantly, this includes RPCs
which do repo optimizations, change config, maintain object pools and
the transaction voting RPCs themselves.

So let's group together all calls which fall into the same category as a
preparatory step to keep the actual code changes small. This shouldn't
yet trigger any change in behaviour.

1 files changed, 46 insertions, 40 deletions

diff --git a/internal/praefect/coordinator.go b/internal/praefect/coordinator.go
index c5f9963b6..ca0bf14db 100644
--- a/internal/praefect/coordinator.go
+++ b/internal/praefect/coordinator.go
@@ -38,45 +38,59 @@ func transactionsDisabled(context.Context) bool { return false }
 // transactions. An optional feature flag can be added to conditionally enable transactional
 // behaviour. If none is given, it's always enabled.
 var transactionRPCs = map[string]transactionsCondition{
-	"/gitaly.CleanupService/ApplyBfgObjectMapStream":           transactionsDisabled,
-	"/gitaly.ConflictsService/ResolveConflicts":                transactionsDisabled,
+	"/gitaly.CleanupService/ApplyBfgObjectMapStream":         transactionsDisabled,
+	"/gitaly.ConflictsService/ResolveConflicts":              transactionsDisabled,
+	"/gitaly.ObjectPoolService/FetchIntoObjectPool":          transactionsDisabled,
+	"/gitaly.OperationService/UserApplyPatch":                transactionsDisabled,
+	"/gitaly.OperationService/UserCherryPick":                transactionsDisabled,
+	"/gitaly.OperationService/UserCommitFiles":               transactionsDisabled,
+	"/gitaly.OperationService/UserFFBranch":                  transactionsDisabled,
+	"/gitaly.OperationService/UserMergeBranch":               transactionsDisabled,
+	"/gitaly.OperationService/UserMergeToRef":                transactionsDisabled,
+	"/gitaly.OperationService/UserRebaseConfirmable":         transactionsDisabled,
+	"/gitaly.OperationService/UserRevert":                    transactionsDisabled,
+	"/gitaly.OperationService/UserSquash":                    transactionsDisabled,
+	"/gitaly.OperationService/UserUpdateSubmodule":           transactionsDisabled,
+	"/gitaly.RefService/DeleteRefs":                          transactionsDisabled,
+	"/gitaly.RemoteService/AddRemote":                        transactionsDisabled,
+	"/gitaly.RemoteService/FetchInternalRemote":              transactionsDisabled,
+	"/gitaly.RemoteService/RemoveRemote":                     transactionsDisabled,
+	"/gitaly.RemoteService/UpdateRemoteMirror":               transactionsDisabled,
+	"/gitaly.RepositoryService/CloneFromPool":                transactionsDisabled,
+	"/gitaly.RepositoryService/CloneFromPoolInternal":        transactionsDisabled,
+	"/gitaly.RepositoryService/CreateFork":                   transactionsDisabled,
+	"/gitaly.RepositoryService/CreateRepositoryFromBundle":   transactionsDisabled,
+	"/gitaly.RepositoryService/CreateRepositoryFromSnapshot": transactionsDisabled,
+	"/gitaly.RepositoryService/CreateRepositoryFromURL":      transactionsDisabled,
+	"/gitaly.RepositoryService/FetchRemote":                  transactionsDisabled,
+	"/gitaly.RepositoryService/FetchSourceBranch":            transactionsDisabled,
+	"/gitaly.RepositoryService/ReplicateRepository":          transactionsDisabled,
+	"/gitaly.RepositoryService/WriteRef":                     transactionsDisabled,
+	"/gitaly.WikiService/WikiDeletePage":                     transactionsDisabled,
+	"/gitaly.WikiService/WikiUpdatePage":                     transactionsDisabled,
+	"/gitaly.WikiService/WikiWritePage":                      transactionsDisabled,
+
+	"/gitaly.OperationService/UserCreateBranch": transactionsEnabled,
+	"/gitaly.OperationService/UserCreateTag":    transactionsEnabled,
+	"/gitaly.OperationService/UserDeleteBranch": transactionsEnabled,
+	"/gitaly.OperationService/UserDeleteTag":    transactionsEnabled,
+	"/gitaly.OperationService/UserUpdateBranch": transactionsEnabled,
+	"/gitaly.SSHService/SSHReceivePack":         transactionsEnabled,
+	"/gitaly.SmartHTTPService/PostReceivePack":  transactionsEnabled,
+
+	// The following RPCs don't perform any reference updates and thus
+	// shouldn't use transactions.
 	"/gitaly.ObjectPoolService/CreateObjectPool":               transactionsDisabled,
 	"/gitaly.ObjectPoolService/DeleteObjectPool":               transactionsDisabled,
 	"/gitaly.ObjectPoolService/DisconnectGitAlternates":        transactionsDisabled,
-	"/gitaly.ObjectPoolService/FetchIntoObjectPool":            transactionsDisabled,
 	"/gitaly.ObjectPoolService/LinkRepositoryToObjectPool":     transactionsDisabled,
 	"/gitaly.ObjectPoolService/ReduplicateRepository":          transactionsDisabled,
 	"/gitaly.ObjectPoolService/UnlinkRepositoryFromObjectPool": transactionsDisabled,
-	"/gitaly.OperationService/UserApplyPatch":                  transactionsDisabled,
-	"/gitaly.OperationService/UserCherryPick":                  transactionsDisabled,
-	"/gitaly.OperationService/UserCommitFiles":                 transactionsDisabled,
-	"/gitaly.OperationService/UserFFBranch":                    transactionsDisabled,
-	"/gitaly.OperationService/UserMergeBranch":                 transactionsDisabled,
-	"/gitaly.OperationService/UserMergeToRef":                  transactionsDisabled,
-	"/gitaly.OperationService/UserRebaseConfirmable":           transactionsDisabled,
-	"/gitaly.OperationService/UserRevert":                      transactionsDisabled,
-	"/gitaly.OperationService/UserSquash":                      transactionsDisabled,
-	"/gitaly.OperationService/UserUpdateSubmodule":             transactionsDisabled,
-	"/gitaly.RefService/DeleteRefs":                            transactionsDisabled,
 	"/gitaly.RefService/PackRefs":                              transactionsDisabled,
-	"/gitaly.RefTransaction/StopTransaction":                   transactionsDisabled,
-	"/gitaly.RefTransaction/VoteTransaction":                   transactionsDisabled,
-	"/gitaly.RemoteService/AddRemote":                          transactionsDisabled,
-	"/gitaly.RemoteService/FetchInternalRemote":                transactionsDisabled,
-	"/gitaly.RemoteService/RemoveRemote":                       transactionsDisabled,
-	"/gitaly.RemoteService/UpdateRemoteMirror":                 transactionsDisabled,
 	"/gitaly.RepositoryService/ApplyGitattributes":             transactionsDisabled,
 	"/gitaly.RepositoryService/Cleanup":                        transactionsDisabled,
-	"/gitaly.RepositoryService/CloneFromPool":                  transactionsDisabled,
-	"/gitaly.RepositoryService/CloneFromPoolInternal":          transactionsDisabled,
-	"/gitaly.RepositoryService/CreateFork":                     transactionsDisabled,
 	"/gitaly.RepositoryService/CreateRepository":               transactionsDisabled,
-	"/gitaly.RepositoryService/CreateRepositoryFromBundle":     transactionsDisabled,
-	"/gitaly.RepositoryService/CreateRepositoryFromSnapshot":   transactionsDisabled,
-	"/gitaly.RepositoryService/CreateRepositoryFromURL":        transactionsDisabled,
 	"/gitaly.RepositoryService/DeleteConfig":                   transactionsDisabled,
-	"/gitaly.RepositoryService/FetchRemote":                    transactionsDisabled,
-	"/gitaly.RepositoryService/FetchSourceBranch":              transactionsDisabled,
 	"/gitaly.RepositoryService/Fsck":                           transactionsDisabled,
 	"/gitaly.RepositoryService/GarbageCollect":                 transactionsDisabled,
 	"/gitaly.RepositoryService/MidxRepack":                     transactionsDisabled,
@@ -85,22 +99,14 @@ var transactionRPCs = map[string]transactionsCondition{
 	"/gitaly.RepositoryService/RenameRepository":               transactionsDisabled,
 	"/gitaly.RepositoryService/RepackFull":                     transactionsDisabled,
 	"/gitaly.RepositoryService/RepackIncremental":              transactionsDisabled,
-	"/gitaly.RepositoryService/ReplicateRepository":            transactionsDisabled,
 	"/gitaly.RepositoryService/RestoreCustomHooks":             transactionsDisabled,
 	"/gitaly.RepositoryService/SetConfig":                      transactionsDisabled,
 	"/gitaly.RepositoryService/WriteCommitGraph":               transactionsDisabled,
-	"/gitaly.RepositoryService/WriteRef":                       transactionsDisabled,
-	"/gitaly.WikiService/WikiDeletePage":                       transactionsDisabled,
-	"/gitaly.WikiService/WikiUpdatePage":                       transactionsDisabled,
-	"/gitaly.WikiService/WikiWritePage":                        transactionsDisabled,
 
-	"/gitaly.OperationService/UserCreateBranch": transactionsEnabled,
-	"/gitaly.OperationService/UserCreateTag":    transactionsEnabled,
-	"/gitaly.OperationService/UserDeleteBranch": transactionsEnabled,
-	"/gitaly.OperationService/UserDeleteTag":    transactionsEnabled,
-	"/gitaly.OperationService/UserUpdateBranch": transactionsEnabled,
-	"/gitaly.SSHService/SSHReceivePack":         transactionsEnabled,
-	"/gitaly.SmartHTTPService/PostReceivePack":  transactionsEnabled,
+	// These shouldn't ever use transactions for the sake of not creating
+	// cyclic dependencies.
+	"/gitaly.RefTransaction/StopTransaction": transactionsDisabled,
+	"/gitaly.RefTransaction/VoteTransaction": transactionsDisabled,
 }
 
 func init() {