diff options
| author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-06-22 09:34:15 +0300 |
|---|---|---|
| committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-07-16 10:13:42 +0300 |
| commit | 60faf845f3a205b198d9bc8893d1833f35addc16 (patch) | |
| tree | fe4879b649c4ae2dea9867c3840057b977c0a6e3 | |
| parent | b4f24cacf673ee2837bb7e39bd92e69919e396c5 (diff) | |
coordinator: Clarify why transactional behaviour is disabled for RPCs
The map which specifies mutating RPCs and whether they should use
transactions or not currently claims that the reason for disabled
transactional behaviour is that the RPCs at hand simply don't modify
references and thus shouldn't use transactions. This line of reasoning
doesn't hold anymore: we have several RPCs which don't modify refs, but
instead perform manual voting on different state.
Split up the list of disabled RPCs into two:
- The first list contains all the RPCs where we didn't implement
manual voting logic yet, but which may be changed in the future to
support transactional behaviour.
- The second list contains RPCs which are considered idempotent
"optimizing" RPCs: while they do modify on-disk state, the result
as visible to the user should always remain the same. Those RPCs
are special cased to not cause a repository generation bump and
thus retaining non-transactional behaviour is fine there.
This should hopefully represent the current state much more accurately.
| -rw-r--r-- | internal/praefect/coordinator.go | 30 |
1 files changed, 17 insertions, 13 deletions
diff --git a/internal/praefect/coordinator.go b/internal/praefect/coordinator.go index 6a372d34a..8e02f9d69 100644 --- a/internal/praefect/coordinator.go +++ b/internal/praefect/coordinator.go @@ -91,28 +91,32 @@ var transactionRPCs = map[string]transactionsCondition{ "/gitaly.RepositoryService/SetConfig": transactionsFlag(featureflag.TxConfig), "/gitaly.RepositoryService/DeleteConfig": transactionsFlag(featureflag.TxConfig), - // The following RPCs don't perform any reference updates and thus - // shouldn't use transactions. + // The following RPCs currently aren't transactional, but we may consider making them + // transactional in the future if the need arises. "/gitaly.ObjectPoolService/CreateObjectPool": transactionsDisabled, "/gitaly.ObjectPoolService/DeleteObjectPool": transactionsDisabled, "/gitaly.ObjectPoolService/DisconnectGitAlternates": transactionsDisabled, "/gitaly.ObjectPoolService/LinkRepositoryToObjectPool": transactionsDisabled, "/gitaly.ObjectPoolService/ReduplicateRepository": transactionsDisabled, "/gitaly.ObjectPoolService/UnlinkRepositoryFromObjectPool": transactionsDisabled, - "/gitaly.RefService/PackRefs": transactionsDisabled, - "/gitaly.RepositoryService/Cleanup": transactionsDisabled, - "/gitaly.RepositoryService/GarbageCollect": transactionsDisabled, - "/gitaly.RepositoryService/MidxRepack": transactionsDisabled, - "/gitaly.RepositoryService/OptimizeRepository": transactionsDisabled, "/gitaly.RepositoryService/RemoveRepository": transactionsDisabled, "/gitaly.RepositoryService/RenameRepository": transactionsDisabled, - "/gitaly.RepositoryService/RepackFull": transactionsDisabled, - "/gitaly.RepositoryService/RepackIncremental": transactionsDisabled, - "/gitaly.RepositoryService/RestoreCustomHooks": transactionsDisabled, - "/gitaly.RepositoryService/WriteCommitGraph": transactionsDisabled, - // These shouldn't ever use transactions for the sake of not creating - // cyclic dependencies. + // The following list of RPCs are considered idempotent RPCs: while they write into the + // target repository, this shouldn't ever have any user-visible impact given that they're + // purely optimizations of the on-disk state. These RPCs are thus treated specially and + // shouldn't ever cause a repository generation bump. + "/gitaly.RefService/PackRefs": transactionsDisabled, + "/gitaly.RepositoryService/Cleanup": transactionsDisabled, + "/gitaly.RepositoryService/GarbageCollect": transactionsDisabled, + "/gitaly.RepositoryService/MidxRepack": transactionsDisabled, + "/gitaly.RepositoryService/OptimizeRepository": transactionsDisabled, + "/gitaly.RepositoryService/RepackFull": transactionsDisabled, + "/gitaly.RepositoryService/RepackIncremental": transactionsDisabled, + "/gitaly.RepositoryService/RestoreCustomHooks": transactionsDisabled, + "/gitaly.RepositoryService/WriteCommitGraph": transactionsDisabled, + + // These shouldn't ever use transactions for the sake of not creating cyclic dependencies. "/gitaly.RefTransaction/StopTransaction": transactionsDisabled, "/gitaly.RefTransaction/VoteTransaction": transactionsDisabled, } |