diff --git a/changelogs/unreleased/enable-catfile-cache-in-test.yml b/changelogs/unreleased/enable-catfile-cache-in-test.yml new file mode 100644 index 0000000000000000000000000000000000000000..3195ac224177fcafccf8ab57888b4c0b83928f2f --- /dev/null +++ b/changelogs/unreleased/enable-catfile-cache-in-test.yml @@ -0,0 +1,5 @@ +--- +title: Use cat-file caching in tests by default +merge_request: 1239 +author: +type: other diff --git a/internal/command/command.go b/internal/command/command.go index 5f72fec602a26b356cb83f0257a3e968607fd68c..4e837132291bbed2e6708de04984c6514b2c0d51 100644 --- a/internal/command/command.go +++ b/internal/command/command.go @@ -134,12 +134,22 @@ func GitPath() string { return config.Config.Git.BinPath } -var wg = &sync.WaitGroup{} +var ( + wg = &sync.WaitGroup{} + cleanupFunctions []func() +) + +// RegisterCleanup registers callbacks that will be run during WaitAllDone. +func RegisterCleanup(fn func()) { cleanupFunctions = append(cleanupFunctions, fn) } // WaitAllDone waits for all commands started by the command package to // finish. This can only be called once in the lifecycle of the current // Go process. func WaitAllDone() { + for _, fn := range cleanupFunctions { + fn() + } + wg.Wait() } diff --git a/internal/git/catfile/batch_cache.go b/internal/git/catfile/batch_cache.go index 10ea489a1d9ef2f1521aa7c4931cc33e9214fc4b..1ae63435557362c8434612871bb1b205b5dcc95e 100644 --- a/internal/git/catfile/batch_cache.go +++ b/internal/git/catfile/batch_cache.go @@ -6,6 +6,7 @@ import ( "time" "github.com/prometheus/client_golang/prometheus" + "gitlab.com/gitlab-org/gitaly/internal/command" "gitlab.com/gitlab-org/gitaly/internal/git/repository" ) @@ -19,18 +20,21 @@ const ( defaultEvictionInterval = 1 * time.Second ) -var catfileCacheMembers = prometheus.NewGauge( - prometheus.GaugeOpts{ - Name: "gitaly_catfile_cache_members", - Help: "Gauge of catfile cache members", - }, -) +var ( + catfileCacheMembers = prometheus.NewGauge( + prometheus.GaugeOpts{ + Name: "gitaly_catfile_cache_members", + Help: "Gauge of catfile cache members", + }, + ) -var cache *batchCache + cache *batchCache +) func init() { prometheus.MustRegister(catfileCacheMembers) cache = newCache(DefaultBatchfileTTL, CacheMaxItems) + command.RegisterCleanup(cache.EvictAll) } func newCacheKey(sessionID string, repo repository.GitRepo) key { diff --git a/internal/git/catfile/catfile.go b/internal/git/catfile/catfile.go index 288470170e3858241f32ecef6afca49baa134fd6..e64d642307796e92b5179718d0d67a7553fdd0b2 100644 --- a/internal/git/catfile/catfile.go +++ b/internal/git/catfile/catfile.go @@ -34,12 +34,6 @@ var totalCatfileProcesses = prometheus.NewCounter( }, ) -const ( - // CacheFeatureFlagKey is the feature flag key for catfile batch caching. This should match - // what is in gitlab-ce - CacheFeatureFlagKey = "catfile-cache" -) - func init() { prometheus.MustRegister(catfileCacheCounter) prometheus.MustRegister(currentCatfileProcesses) @@ -135,7 +129,7 @@ func New(ctx context.Context, repo *gitalypb.Repository) (*Batch, error) { sessionID := metadata.GetValue(ctx, "gitaly-session-id") - if featureflag.IsDisabled(ctx, CacheFeatureFlagKey) || sessionID == "" { + if featureflag.IsDisabled(ctx, featureflag.CatfileCacheKey) || sessionID == "" { return newBatch(ctx, repoPath, env) } diff --git a/internal/metadata/featureflag/flags.go b/internal/metadata/featureflag/flags.go new file mode 100644 index 0000000000000000000000000000000000000000..9ca6f7520e882bd555b21a6dd895409b3aa95026 --- /dev/null +++ b/internal/metadata/featureflag/flags.go @@ -0,0 +1,10 @@ +package featureflag + +const ( + // CatfileCacheKey is the feature flag key for catfile batch caching. This should match + // what is in gitlab-ce + CatfileCacheKey = "catfile-cache" + + // DeltaIslandsKey controls whether we use Git delta islands during a repack. + DeltaIslandsKey = "delta-islands" +) diff --git a/internal/service/commit/find_commit_test.go b/internal/service/commit/find_commit_test.go index ab64427c70f8bf699004fb9f59c1c82d07ac820b..c7a6837d43070ecb895107fe047dfbf75bc2495f 100644 --- a/internal/service/commit/find_commit_test.go +++ b/internal/service/commit/find_commit_test.go @@ -332,7 +332,7 @@ func benchmarkFindCommit(withCache bool, b *testing.B) { revision := revisions[b.N%len(revisions)] if withCache { md := metadata.New(map[string]string{ - featureflag.HeaderKey(catfile.CacheFeatureFlagKey): "true", + featureflag.HeaderKey(featureflag.CatfileCacheKey): "true", "gitaly-session-id": "abc123", }) @@ -378,7 +378,7 @@ func TestFindCommitWithCache(t *testing.T) { for i := 0; i < 10; i++ { revision := revisions[i%len(revisions)] md := metadata.New(map[string]string{ - featureflag.HeaderKey(catfile.CacheFeatureFlagKey): "true", + featureflag.HeaderKey(featureflag.CatfileCacheKey): "true", "gitaly-session-id": "abc123", }) diff --git a/internal/service/repository/gc_test.go b/internal/service/repository/gc_test.go index 7bd0969af311a1d956967b8d289a2998d3bfd5a3..1ecd8689c40f50ac09b2ee58955c4f9288ad5d47 100644 --- a/internal/service/repository/gc_test.go +++ b/internal/service/repository/gc_test.go @@ -309,7 +309,7 @@ func TestGarbageCollectDeltaIslands(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - md := metadata.New(map[string]string{featureflag.HeaderKey(deltaIslandsFeatureFlag): "true"}) + md := metadata.New(map[string]string{featureflag.HeaderKey(featureflag.DeltaIslandsKey): "true"}) ctxWithFeatureFlag := metadata.NewOutgoingContext(ctx, md) testCases := []struct { diff --git a/internal/service/repository/repack.go b/internal/service/repository/repack.go index aeaf5fe4f1016bbe99abcec97a19efe4a5b6496b..8af27f39579cfc5baaba27202a9d8dc2b5fed5a9 100644 --- a/internal/service/repository/repack.go +++ b/internal/service/repository/repack.go @@ -13,8 +13,6 @@ import ( "google.golang.org/grpc/status" ) -const deltaIslandsFeatureFlag = "delta-islands" - var ( repackCounter = prometheus.NewCounterVec( prometheus.CounterOpts{ @@ -73,7 +71,7 @@ func repackConfig(ctx context.Context, bitmap bool) []string { args = append(args, "-c", "repack.writeBitmaps=false") } - deltaIslands := featureflag.IsEnabled(ctx, deltaIslandsFeatureFlag) + deltaIslands := featureflag.IsEnabled(ctx, featureflag.DeltaIslandsKey) if deltaIslands { args = append(args, "-c", "pack.island=refs/heads", diff --git a/internal/service/repository/repack_test.go b/internal/service/repository/repack_test.go index 082d41243d560090f95035408bdcf4e220668ba5..237f4bfa0ff8a3805d185ab7c9a6dd2234bd8a43 100644 --- a/internal/service/repository/repack_test.go +++ b/internal/service/repository/repack_test.go @@ -204,7 +204,7 @@ func TestRepackFullDeltaIslands(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - md := metadata.New(map[string]string{featureflag.HeaderKey(deltaIslandsFeatureFlag): "true"}) + md := metadata.New(map[string]string{featureflag.HeaderKey(featureflag.DeltaIslandsKey): "true"}) ctxWithFeatureFlag := metadata.NewOutgoingContext(ctx, md) testCases := []struct { diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index 5b139227f4c8948a86368c871904a26ce040e06b..c3d273503dd10739ac63b07ee32abdb5976cbb71 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -31,6 +31,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/helper/fieldextractors" "gitlab.com/gitlab-org/gitaly/internal/helper/text" gitalylog "gitlab.com/gitlab-org/gitaly/internal/log" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/storage" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -348,7 +349,15 @@ func mustFindNoRunningChildProcess() { // Context returns a cancellable context. func Context() (context.Context, func()) { - return context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(context.Background()) + + md := metadata.New(map[string]string{ + featureflag.HeaderKey(featureflag.CatfileCacheKey): "true", + "gitaly-session-id": "gitaly-test", + }) + ctx = metadata.NewOutgoingContext(ctx, md) + + return ctx, cancel } // CreateRepo creates an temporary directory for a repo, without initializing it