diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-07-14 14:54:39 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-07-14 15:04:07 +0300 |
commit | 608d1a1124ca4f5290910e3c1d1a59784f07f3f3 (patch) | |
tree | 25c424f78ea831813ac06f21b265381ddc982870 | |
parent | 76dabc8174f7978025f48adcfab0a19c85416531 (diff) |
maintenance: Remove per-repository timeouts in nightly maintenance
Our nightly maintenance job is running in a time-limited window of 30
minutes by default. Despite this timeout we have in place for the
complete maintenance job we also have a per-repository timeout of 5
minutes that cannot be altered at all. This timeout exists so that as
many repositories as possible have the chance to get optimized.
In total this per-repository timeout creates more harm than benefit
though. While it may in fact cause us to optimize more repositories, it
also has the effect that we may consistently fail to optimize any repos
which are bigger than a certain threshold because they take more than 5
minutes to optimize. So this timeout prioritizes small repositories and
penalizes huge repositories.
This is indeed nothing we want. In fact, I'd even argue that maintaining
repositories that are this huge is a lot more important than maintaining
the smaller ones given that they're so much more expensive to serve.
Drop the per-repository timeout to stop penalizing large repositories.
Optimizing them will still be cancelled when the overall timeframe for
the nightly maintenance job is over.
Changelog: fixed
-rw-r--r-- | internal/gitaly/maintenance/optimize.go | 3 | ||||
-rw-r--r-- | internal/gitaly/maintenance/optimize_test.go | 38 |
2 files changed, 1 insertions, 40 deletions
diff --git a/internal/gitaly/maintenance/optimize.go b/internal/gitaly/maintenance/optimize.go index f554e33f5..a692613d8 100644 --- a/internal/gitaly/maintenance/optimize.go +++ b/internal/gitaly/maintenance/optimize.go @@ -116,8 +116,7 @@ func optimizeRepoAtPath(ctx context.Context, l logrus.FieldLogger, s config.Stor "start_time": start.UTC(), }) - ctx, cancel := context.WithTimeout(ctxlogrus.ToContext(ctx, logEntry), 5*time.Minute) - defer cancel() + ctx = ctxlogrus.ToContext(ctx, logEntry) err = o.OptimizeRepository(ctx, repo) logEntry = logEntry.WithField("time_ms", time.Since(start).Milliseconds()) diff --git a/internal/gitaly/maintenance/optimize_test.go b/internal/gitaly/maintenance/optimize_test.go index dd4bd2b3e..5a5ba3a0f 100644 --- a/internal/gitaly/maintenance/optimize_test.go +++ b/internal/gitaly/maintenance/optimize_test.go @@ -5,9 +5,7 @@ import ( "math/rand" "path/filepath" "testing" - "time" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/backchannel" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" @@ -110,39 +108,3 @@ func TestOptimizeReposRandomly(t *testing.T) { }) } } - -type mockOptimizerCancel struct { - t *testing.T - startedAt time.Time -} - -func (m mockOptimizerCancel) OptimizeRepository(ctx context.Context, _ repo.GitRepo) error { - timeline, ok := ctx.Deadline() - if assert.True(m.t, ok) { - assert.True(m.t, timeline.After(m.startedAt), m.startedAt) - future := m.startedAt.Add(10 * time.Minute) - assert.True(m.t, timeline.Before(future), future) - } - return nil -} - -func TestOptimizeReposRandomly_cancellationOverride(t *testing.T) { - cfgBuilder := testcfg.NewGitalyCfgBuilder() - cfg := cfgBuilder.Build(t) - - gittest.InitRepo(t, cfg, cfg.Storages[0]) - ctx := testhelper.Context(t) - - // The timeout should be overwritten by the default 5 min timeout. - //nolint:forbidigo // We're explicitly testing deadline override. - ctx, cancel := context.WithTimeout(ctx, 72*time.Hour) - defer cancel() - - ticker := helper.NewManualTicker() - ticker.Tick() - - mo := &mockOptimizerCancel{t: t, startedAt: time.Now()} - walker := OptimizeReposRandomly(cfg.Storages, mo, ticker, rand.New(rand.NewSource(1))) - - require.NoError(t, walker(ctx, testhelper.NewDiscardingLogEntry(t), []string{cfg.Storages[0].Name})) -} |