Make UpdateRepositoryStorageService idempotent
What does this MR do?
This fixes a data loss scenario that can occur when moving a repository between Gitaly shards ("storages"). The code that moves the repository soft-deletes the repository from its old location. If the new location and the old location are identical (i.e. have the same storage name), you end up losing your repository.
We already check for the "old storage equals new storage" in some places but that was not enough.
This bug was happening for me because I was in a case where the repository move job itself would fail halfway through, and got rescheduled by Sidekiq. That caused it to run a second time with the exact same inputs leading to data loss. This was not obvious to spot in development because the retry only happened after 1 minute.
Anyway, with this change we add a guard that checks if the old and new location are the same, and if they are, we bail out.
What are the relevant issue numbers?
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary -
Documentation created/updated via this MR -
Documentation reviewed by technical writer or follow-up review issue created -
Tests added for this feature/bug -
Tested in all supported browsers -
Conforms to the code review guidelines -
Conforms to the merge request performance guidelines -
Conforms to the style guides -
Conforms to the database guides -
Link to e2e tests MR added if this MR has Requires e2e tests label. See the Test Planning Process. -
EE specific content should be in the top level /eefolder -
For a paid feature, have we considered GitLab.com plans, how it works for groups, and is there a design for promoting it to users who aren't on the correct plan? -
Security reports checked/validated by reviewer