diff options
| author | Sami Hiltunen <shiltunen@gitlab.com> | 2022-03-07 20:01:30 +0300 |
|---|---|---|
| committer | Sami Hiltunen <shiltunen@gitlab.com> | 2022-03-07 20:01:30 +0300 |
| commit | ce2e5ead35c404ecf3bfa7443637e9a3b5aed231 (patch) | |
| tree | a6c5fbd05b249d1ba838393eeb8ae137b70d6798 | |
| parent | a928ebb0951f52bfccbfc2e67543abe8bc3f3963 (diff) | |
Extend invalid metadata deletion logic to repos existin on targetsmh-extend-invalid-source-repository
Praefect contains logic to delete metadata record of a non-existent
source repository during replication. Praefect checks for the
ErrInvalidSourceRepository error, and if it receives it, it deletes
the metadata record of the source repository. Right now, the error
is only returned from ReplicateRepository if the source repository
didn't exist when the target is creating the repository for the
first time. This means Praefect's clean up logic doesn't run if the
target already has the repository on the disk but the source repository
doesn't exist. This can lead to reconciliation loops where Praefect
keeps trying to replicate from the non-existent source repository
over and over again as all of the replications fail. This commit
changes ReplicateRepository to return the error when the source
doesn't exist but the target repository does.
This logic was initially implemented to break the reonciliation loops
that occurred due to Praefect creating invalid metadata records when
a repository creation failed. As DeleteObjectPool RPC was not handled
as a repository deleting RPC in Praefect, the metadata records were
left in place which is currently causing reconciliation loops. Changing
the error return allows for Praefect to clean up those invalid metadata
records. In the long run, Praefect's background metadata verifier would
capture such issues and this logic could be removed.
Changelog: fixed
| -rw-r--r-- | internal/gitaly/service/repository/replicate.go | 2 | ||||
| -rw-r--r-- | internal/gitaly/service/repository/replicate_test.go | 3 |
2 files changed, 2 insertions, 3 deletions
diff --git a/internal/gitaly/service/repository/replicate.go b/internal/gitaly/service/repository/replicate.go index b2da1d4ae..847c4ece8 100644 --- a/internal/gitaly/service/repository/replicate.go +++ b/internal/gitaly/service/repository/replicate.go @@ -70,7 +70,7 @@ func (s *server) ReplicateRepository(ctx context.Context, in *gitalypb.Replicate return nil, helper.ErrInternalf("checking for repo existence: %w", err) } if !request.GetExists() { - return nil, helper.ErrNotFoundf("source repository does not exist") + return nil, ErrInvalidSourceRepository } outgoingCtx := metadata.IncomingToOutgoing(ctx) diff --git a/internal/gitaly/service/repository/replicate_test.go b/internal/gitaly/service/repository/replicate_test.go index 7b87f8923..07d011d18 100644 --- a/internal/gitaly/service/repository/replicate_test.go +++ b/internal/gitaly/service/repository/replicate_test.go @@ -26,7 +26,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/ssh" "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v14/internal/helper" "gitlab.com/gitlab-org/gitaly/v14/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v14/internal/metadata" "gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag" @@ -324,7 +323,7 @@ func testReplicateRepositoryBadRepository(t *testing.T, ctx context.Context) { desc: "source invalid", invalidSource: true, error: func(t testing.TB, actual error) { - testhelper.RequireGrpcError(t, actual, helper.ErrNotFoundf("source repository does not exist")) + testhelper.RequireGrpcError(t, actual, ErrInvalidSourceRepository) }, }, { |