Skip to content

repository: Implement transactional support for RenameRepository

Patrick Steinhardt requested to merge pks-tx-rename-repository into master

The RenameRepository RPC does not have any support for transactions right now. This is causing problems for Geo setups which use Gitaly Cluster given that Geo uses renames quite a lot. As a result, whenever Geo renames any repository, it will create replication jobs and thus put quite some additional load on the nodes.

Implement transactional support in RenameRepository. The schema we full is the same as we do for CreateRepository et al.:

1. We assert that the source repository exists and that the target
   repository does not exist. This is done up front to be able to
   error out fast in case any of the prerequisites isn't given.

2. We lock both the source repository and the target repository.
   This is the same kind of lock as used by CreateReposiotry et al
   and will thus cause concurrent creates of the target repo to
   fail.

3. Assert again that the target repository does not exist. We do
   this again after having taken the lock because something might've
   created it before we took the lock.

4. We do a preparatory vote to determine whether all Gitalies can
   commit.

5. The repository is renamed into place.

6. A committing vote is performed to show that all Gitalies have
   performed the change.

With this schema, the critical section is as short as possible while we can be sufficiently sure that no other RPC call would interfere with the non-existing target repository.

What this explicitly does not protect against is concurrent modifications of the repository for any in-flight RPC calls. So it can happen that the repository, after having been moved, is in a different state across nodes. To detect this, we'd have to scan the repository's contents to compute a vote, which can be expensive depending on the repo's size. And given that the rename itself is cheap, the additional latency is not something we'd likely want to have for now. Any subsequent transactions would note if references have diverged anyway, so this should be good enough for now.

Add a new feature flag for this flag and teach Praefect to respect this flag as well.

Changelog: changed

Closes #3832 (closed)

Merge request reports