Fork creation fails with transactions enabled
Fork creation with transactions currently fails due to the fork repository ending up in a different partition from the origin repository and the pool. This happens due to Rails sending Gitaly a RepositoryExists
requests before sending a CreateFork
request.
As the RepositoryExists
request is the first access to the fork repository, it assigns the fork into a partition. Unlike the CreateFork
request, the RepositoryExists
request contains no hint for Gitaly to determine which fork repository should be placed in, so Gitaly assigns it into a new partition. As the fork is now assigned into a different partition than the origin repository, we can't later actually create the fork.
There are a few ways to address this:
- Remove the
RepositoryExists
request and just attempt to create the fork. It's seemingly unnecessary to check first whether the fork already exists when we can just attempt to create it and handle the error. (#5956 (closed)) - Add a partitioning hint to the
RepositoryExists
request so Gitaly will know we want the repository to be partitioned with the origin repository. This is similar approach to Add partitioning hinting to project moves (#5764 - closed). - Fix the partitioning logic to not persist assignments before a repository is created. Relates to #5763.
- Subproblem of this is Don't create partitions assignments when access... (#5957 - closed). Solving this is likely enough to fix fork creation with transactions.
Option 1 makes sense on its own, option 2 is more of a workaround. Option 3 is something we should do anyway but seems to be a fair amount more work than the other two, and can be addressed later as well.
We have a test in Gitaly that seeks to replicate a fork creation flow. Unfortunately it missed this as it wasn't sending the RepositoryExists
request first. We should extend the test to do so.
Below diff adds the missing request to the test case and reproduces the issue.
diff --git a/internal/gitaly/service/objectpool/link_test.go b/internal/gitaly/service/objectpool/link_test.go
index a5e705546..8fa9cc5fe 100644
--- a/internal/gitaly/service/objectpool/link_test.go
+++ b/internal/gitaly/service/objectpool/link_test.go
@@ -39,6 +39,16 @@ func TestCompleteForkCreationFlow(t *testing.T) {
ctx = testhelper.MergeOutgoingMetadata(ctx, testcfg.GitalyServersMetadataFromCfg(t, cfg))
// Build GitalySSH as CreateFork uses to perform the fetch.
testcfg.BuildGitalySSH(t, cfg)
+
+ // Rails sends a RepositoryExists request before creating the fork as well.
+ repositoryExistsResponse, err := repositoryClient.RepositoryExists(ctx, &gitalypb.RepositoryExistsRequest{
+ Repository: forkRepository,
+ })
+ require.NoError(t, err)
+ testhelper.ProtoEqual(t, &gitalypb.RepositoryExistsResponse{
+ Exists: false,
+ }, repositoryExistsResponse)
+
createForkResponse, err := repositoryClient.CreateFork(ctx, &gitalypb.CreateForkRequest{
Repository: forkRepository,
SourceRepository: sourceRepository,