Skip to content

git: Stop calling Gitaly's Cleanup RPC

Patrick Steinhardt requested to merge pks-gitaly-drop-cleanup-calls into master

What does this MR do and why?

Historically, Gitaly easily ended up in states where the repository is in a state such that many different Git commands failed. This is why Gitaly provides a Cleanup RPC that cleans up any such data structures which are known to cause problems, and we call it in many different places.

There had been two main usecases for this:

- Git is known to leave behind corrupted references in case of a
  hard shutdown because it doesn't fsync(3p) data to disk before
  renaming loose references into place. Such broken references may
  cause Git commands to fail which try to read them, and as such
  Gitaly used to perform housekeeping tasks where it walked through
  the repository to clean up any such corrupt references. Gitaly had
  to eventually remove this functionality from Cleanup though: in
  repositories hosted on NFS this was causing significant latency
  and thus caused timeouts. We thus don't do this anymore since
  7a1d224d0 (repository: Remove housekeeping from Cleanup RPC,
  2021-05-17).

- Many of Gitaly's RPCs used to create worktrees to perform various
  operations. In some cases worktrees were pruned, but left behind
  broken references which as a result again caused corrupted
  references. The Cleanup RPC thus prunes any such per-worktree
  references and cleans up pointers to already-removed worktrees.
  Gitaly has since converted all RPCs except UserApplyPatch to not
  use worktrees anymore, and this remaining RPC is effectively
  unused: this week we had two calls of it across all of gitlab.com.

It is thus safe to say that the Cleanup RPC is effectively useless and doesn't do anything. It doesn't clean up corrupted references, and because we don't create worktrees we wouldn't ever need to clean them up either. Furthermore, both of those actions are performed by other RPCs, namely GarbageCollect and OptimizeRepository. The former one is frequently executed by Rails, while the latter one is automatically executed during our daily maintenance window on all repositories.

Let's thus stop calling Cleanup altogether such that its RPC can be properly deprecated and removed in Gitaly.

Part of gitaly#4039 (closed)

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Patrick Steinhardt

Merge request reports