Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--internal/git/objectpool/fetch.go7
-rw-r--r--internal/git/objectpool/fetch_test.go38
-rw-r--r--internal/gitaly/service/objectpool/fetch_into_object_pool_test.go106
-rw-r--r--internal/metadata/featureflag/ff_fetch_into_object_pool_prune_refs.go11
4 files changed, 37 insertions, 125 deletions
diff --git a/internal/git/objectpool/fetch.go b/internal/git/objectpool/fetch.go
index 0b399dc13..7f3e72472 100644
--- a/internal/git/objectpool/fetch.go
+++ b/internal/git/objectpool/fetch.go
@@ -17,7 +17,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo"
"gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref"
"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction"
- "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
"gitlab.com/gitlab-org/gitaly/v15/internal/transaction/voting"
)
@@ -58,10 +57,8 @@ func (o *ObjectPool) FetchFromOrigin(ctx context.Context, origin *localrepo.Repo
// `--atomic` and `--prune` together then it still wouldn't be able to recover from the D/F
// conflict. So we first to a preliminary prune that only prunes refs without fetching
// objects yet to avoid that scenario.
- if featureflag.FetchIntoObjectPoolPruneRefs.IsEnabled(ctx) {
- if err := o.pruneReferences(ctx, origin); err != nil {
- return fmt.Errorf("pruning references: %w", err)
- }
+ if err := o.pruneReferences(ctx, origin); err != nil {
+ return fmt.Errorf("pruning references: %w", err)
}
var stderr bytes.Buffer
diff --git a/internal/git/objectpool/fetch_test.go b/internal/git/objectpool/fetch_test.go
index 0a03970ec..7305697f0 100644
--- a/internal/git/objectpool/fetch_test.go
+++ b/internal/git/objectpool/fetch_test.go
@@ -3,7 +3,6 @@
package objectpool
import (
- "context"
"fmt"
"os"
"path/filepath"
@@ -17,18 +16,13 @@ import (
"gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo"
"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
"gitlab.com/gitlab-org/gitaly/v15/internal/helper/text"
- "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
)
func TestFetchFromOrigin_dangling(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets(featureflag.FetchIntoObjectPoolPruneRefs).Run(t, testFetchFromOriginDangling)
-}
-
-func testFetchFromOriginDangling(t *testing.T, ctx context.Context) {
- t.Parallel()
+ ctx := testhelper.Context(t)
cfg, pool, repoProto := setupObjectPool(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -98,12 +92,8 @@ func testFetchFromOriginDangling(t *testing.T, ctx context.Context) {
func TestFetchFromOrigin_fsck(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets(featureflag.FetchIntoObjectPoolPruneRefs).Run(t, testFetchFromOriginFsck)
-}
-
-func testFetchFromOriginFsck(t *testing.T, ctx context.Context) {
- t.Parallel()
+ ctx := testhelper.Context(t)
cfg, pool, repoProto := setupObjectPool(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
repoPath := filepath.Join(cfg.Storages[0].Path, repo.GetRelativePath())
@@ -127,12 +117,8 @@ func testFetchFromOriginFsck(t *testing.T, ctx context.Context) {
func TestFetchFromOrigin_deltaIslands(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets(featureflag.FetchIntoObjectPoolPruneRefs).Run(t, testFetchFromOriginDeltaIslands)
-}
-
-func testFetchFromOriginDeltaIslands(t *testing.T, ctx context.Context) {
- t.Parallel()
+ ctx := testhelper.Context(t)
cfg, pool, repoProto := setupObjectPool(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -152,12 +138,8 @@ func testFetchFromOriginDeltaIslands(t *testing.T, ctx context.Context) {
func TestFetchFromOrigin_bitmapHashCache(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets(featureflag.FetchIntoObjectPoolPruneRefs).Run(t, testFetchFromOriginBitmapHashCache)
-}
-
-func testFetchFromOriginBitmapHashCache(t *testing.T, ctx context.Context) {
- t.Parallel()
+ ctx := testhelper.Context(t)
cfg, pool, repoProto := setupObjectPool(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -182,12 +164,8 @@ func testFetchFromOriginBitmapHashCache(t *testing.T, ctx context.Context) {
func TestFetchFromOrigin_refUpdates(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets(featureflag.FetchIntoObjectPoolPruneRefs).Run(t, testFetchFromOriginRefUpdates)
-}
-
-func testFetchFromOriginRefUpdates(t *testing.T, ctx context.Context) {
- t.Parallel()
+ ctx := testhelper.Context(t)
cfg, pool, repoProto := setupObjectPool(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
repoPath := filepath.Join(cfg.Storages[0].Path, repo.GetRelativePath())
@@ -239,12 +217,8 @@ func testFetchFromOriginRefUpdates(t *testing.T, ctx context.Context) {
func TestFetchFromOrigin_refs(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets(featureflag.FetchIntoObjectPoolPruneRefs).Run(t, testFetchFromOriginRefs)
-}
-
-func testFetchFromOriginRefs(t *testing.T, ctx context.Context) {
- t.Parallel()
+ ctx := testhelper.Context(t)
cfg, pool, _ := setupObjectPool(t, ctx)
poolPath := pool.FullPath()
diff --git a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
index 03262537b..582e7180e 100644
--- a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
+++ b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
@@ -22,9 +22,7 @@ import (
"gitlab.com/gitlab-org/gitaly/v15/internal/git/housekeeping"
"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction"
- "gitlab.com/gitlab-org/gitaly/v15/internal/helper"
"gitlab.com/gitlab-org/gitaly/v15/internal/metadata"
- "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver"
@@ -40,12 +38,8 @@ import (
func TestFetchIntoObjectPool_Success(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets(featureflag.FetchIntoObjectPoolPruneRefs).Run(t, testFetchIntoObjectPoolSuccess)
-}
-
-func testFetchIntoObjectPoolSuccess(t *testing.T, ctx context.Context) {
- t.Parallel()
+ ctx := testhelper.Context(t)
cfg, repo, repoPath, locator, client := setup(ctx, t)
repoCommit := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(t.Name()))
@@ -98,11 +92,6 @@ func testFetchIntoObjectPoolSuccess(t *testing.T, ctx context.Context) {
func TestFetchIntoObjectPool_transactional(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets(featureflag.FetchIntoObjectPoolPruneRefs).Run(t, testFetchIntoObjectPoolTransactional)
-}
-
-func testFetchIntoObjectPoolTransactional(t *testing.T, ctx context.Context) {
- t.Parallel()
var votes []voting.Vote
var votesMutex sync.Mutex
@@ -115,6 +104,7 @@ func testFetchIntoObjectPoolTransactional(t *testing.T, ctx context.Context) {
},
}
+ ctx := testhelper.Context(t)
cfg := testcfg.Build(t)
cfg.SocketPath = runObjectPoolServer(
t, cfg, config.NewLocator(cfg),
@@ -163,17 +153,13 @@ func testFetchIntoObjectPoolTransactional(t *testing.T, ctx context.Context) {
})
require.NoError(t, err)
- if featureflag.FetchIntoObjectPoolPruneRefs.IsEnabled(ctx) {
- require.Equal(t, []voting.Vote{
- // We expect to see two votes that demonstrate we're voting on no deleted
- // references.
- voting.VoteFromData(nil), voting.VoteFromData(nil),
- // It is a bug though that we don't have a vote on the unchanged references
- // in git-fetch(1).
- }, votes)
- } else {
- require.Nil(t, votes)
- }
+ require.Equal(t, []voting.Vote{
+ // We expect to see two votes that demonstrate we're voting on no deleted
+ // references.
+ voting.VoteFromData(nil), voting.VoteFromData(nil),
+ // It is a bug though that we don't have a vote on the unchanged references
+ // in git-fetch(1).
+ }, votes)
})
t.Run("with a new reference", func(t *testing.T) {
@@ -192,17 +178,13 @@ func testFetchIntoObjectPoolTransactional(t *testing.T, ctx context.Context) {
vote := voting.VoteFromData([]byte(fmt.Sprintf(
"%s %s refs/remotes/origin/heads/new-branch\n", git.ObjectHashSHA1.ZeroOID, repoCommit,
)))
- if featureflag.FetchIntoObjectPoolPruneRefs.IsEnabled(ctx) {
- require.Equal(t, []voting.Vote{
- // The first two votes stem from the fact that we're voting on no
- // deleted references.
- voting.VoteFromData(nil), voting.VoteFromData(nil),
- // And the other two votes are from the new branch we pull in.
- vote, vote,
- }, votes)
- } else {
- require.Equal(t, []voting.Vote{vote, vote}, votes)
- }
+ require.Equal(t, []voting.Vote{
+ // The first two votes stem from the fact that we're voting on no
+ // deleted references.
+ voting.VoteFromData(nil), voting.VoteFromData(nil),
+ // And the other two votes are from the new branch we pull in.
+ vote, vote,
+ }, votes)
})
t.Run("with a stale reference in pool", func(t *testing.T) {
@@ -220,29 +202,20 @@ func testFetchIntoObjectPoolTransactional(t *testing.T, ctx context.Context) {
})
require.NoError(t, err)
- if featureflag.FetchIntoObjectPoolPruneRefs.IsEnabled(ctx) {
- // We expect a single vote on the reference we have deleted.
- vote := voting.VoteFromData([]byte(fmt.Sprintf(
- "%[1]s %[1]s %s\n", git.ObjectHashSHA1.ZeroOID, reference,
- )))
- require.Equal(t, []voting.Vote{vote, vote}, votes)
- } else {
- require.Nil(t, votes)
- }
+ // We expect a single vote on the reference we have deleted.
+ vote := voting.VoteFromData([]byte(fmt.Sprintf(
+ "%[1]s %[1]s %s\n", git.ObjectHashSHA1.ZeroOID, reference,
+ )))
+ require.Equal(t, []voting.Vote{vote, vote}, votes)
exists, err := pool.Repo.HasRevision(ctx, git.Revision(reference))
require.NoError(t, err)
- require.Equal(t, exists, featureflag.FetchIntoObjectPoolPruneRefs.IsDisabled(ctx))
+ require.False(t, exists)
})
}
func TestFetchIntoObjectPool_CollectLogStatistics(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets(featureflag.FetchIntoObjectPoolPruneRefs).Run(t, testFetchIntoObjectPoolCollectLogStatistics)
-}
-
-func testFetchIntoObjectPoolCollectLogStatistics(t *testing.T, ctx context.Context) {
- t.Parallel()
cfg := testcfg.Build(t)
testcfg.BuildGitalyHooks(t, cfg)
@@ -252,6 +225,7 @@ func testFetchIntoObjectPoolCollectLogStatistics(t *testing.T, ctx context.Conte
logger, hook := test.NewNullLogger()
cfg.SocketPath = runObjectPoolServer(t, cfg, locator, logger)
+ ctx := testhelper.Context(t)
ctx = ctxlogrus.ToContext(ctx, log.WithField("test", "logging"))
repo, _ := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{
Seed: gittest.SeedGitLabTest,
@@ -371,12 +345,8 @@ func TestFetchIntoObjectPool_Failure(t *testing.T) {
func TestFetchIntoObjectPool_dfConflict(t *testing.T) {
t.Parallel()
- testhelper.NewFeatureSets(featureflag.FetchIntoObjectPoolPruneRefs).Run(t, testFetchIntoObjectPoolDfConflict)
-}
-
-func testFetchIntoObjectPoolDfConflict(t *testing.T, ctx context.Context) {
- t.Parallel()
+ ctx := testhelper.Context(t)
cfg, repo, repoPath, _, client := setup(ctx, t)
pool := initObjectPool(t, cfg, cfg.Storages[0])
@@ -401,15 +371,6 @@ func testFetchIntoObjectPoolDfConflict(t *testing.T, ctx context.Context) {
gittest.Exec(t, cfg, "-C", repoPath, "update-ref", "-d", "refs/heads/branch")
gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("branch/conflict"))
- gitVersion, err := gittest.NewCommandFactory(t, cfg).GitVersion(ctx)
- require.NoError(t, err)
-
- // Due to a bug in old Git versions we may get an unexpected exit status.
- expectedExitStatus := 254
- if !gitVersion.FlushesUpdaterefStatus() {
- expectedExitStatus = 1
- }
-
// Verify that we can still fetch into the object pool regardless of the D/F conflict. While
// it is not possible to store both references at the same time due to the conflict, we
// should know to delete the old conflicting reference and replace it with the new one.
@@ -417,20 +378,11 @@ func testFetchIntoObjectPoolDfConflict(t *testing.T, ctx context.Context) {
ObjectPool: pool.ToProto(),
Origin: repo,
})
- if featureflag.FetchIntoObjectPoolPruneRefs.IsEnabled(ctx) {
- require.NoError(t, err)
+ require.NoError(t, err)
- poolPath, err := config.NewLocator(cfg).GetRepoPath(gittest.RewrittenRepository(ctx, t, cfg, pool.ToProto().GetRepository()))
- require.NoError(t, err)
+ poolPath, err := config.NewLocator(cfg).GetRepoPath(gittest.RewrittenRepository(ctx, t, cfg, pool.ToProto().GetRepository()))
+ require.NoError(t, err)
- // Verify that the conflicting reference exists now.
- gittest.Exec(t, cfg, "-C", poolPath, "rev-parse", "refs/remotes/origin/heads/branch/conflict")
- } else {
- // But right now it fails due to a bug.
- testhelper.RequireGrpcError(t, helper.ErrInternalf(
- "fetch into object pool: exit status %d, stderr: %q",
- expectedExitStatus,
- "error: cannot lock ref 'refs/remotes/origin/heads/branch/conflict': 'refs/remotes/origin/heads/branch' exists; cannot create 'refs/remotes/origin/heads/branch/conflict'\n",
- ), err)
- }
+ // Verify that the conflicting reference exists now.
+ gittest.Exec(t, cfg, "-C", poolPath, "rev-parse", "refs/remotes/origin/heads/branch/conflict")
}
diff --git a/internal/metadata/featureflag/ff_fetch_into_object_pool_prune_refs.go b/internal/metadata/featureflag/ff_fetch_into_object_pool_prune_refs.go
deleted file mode 100644
index 4f0a2b41b..000000000
--- a/internal/metadata/featureflag/ff_fetch_into_object_pool_prune_refs.go
+++ /dev/null
@@ -1,11 +0,0 @@
-package featureflag
-
-// FetchIntoObjectPoolPruneRefs enables pruning of references in object pools. This is required in
-// order to fix cases where updating pools doesn't work anymore due to preexisting references
-// conflicting with new references in the pool member.
-var FetchIntoObjectPoolPruneRefs = NewFeatureFlag(
- "fetch_into_object_pool_prune_refs",
- "v15.3.0",
- "https://gitlab.com/gitlab-org/gitaly/-/issues/4394",
- true,
-)
generated by cgit v1.2.3 (git 2.25.1) at 2024-06-03 00:53:31 +0300