From 9430946dfd4c1e645c367379dbeda122aaf084f7 Mon Sep 17 00:00:00 2001 From: Weilu Jia Date: Wed, 1 Jul 2026 16:39:57 -0700 Subject: [PATCH] fix(orchestrator): let build-cache threshold flag raise above its fallback startDiskSpaceEviction seeded the eviction threshold with the flag fallback (85) and only ever lowered it, so a build-cache-max-usage-percentage flag value above 85 was silently capped at the fallback. Extract the computation into evictionThreshold: take the minimum across the configured services' flag values (each of which already falls back inside IntFlag on evaluation failure), and use the fallback directly only when no services are configured. --- .../orchestrator/pkg/sandbox/build/cache.go | 33 ++++++++---- .../pkg/sandbox/build/cache_test.go | 53 +++++++++++++++++++ 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/packages/orchestrator/pkg/sandbox/build/cache.go b/packages/orchestrator/pkg/sandbox/build/cache.go index d6ce891314..d7142106a3 100644 --- a/packages/orchestrator/pkg/sandbox/build/cache.go +++ b/packages/orchestrator/pkg/sandbox/build/cache.go @@ -5,6 +5,7 @@ package build import ( "context" "fmt" + "math" "os" "sync" "time" @@ -222,15 +223,7 @@ func (s *DiffStore) startDiskSpaceEviction( used := int64(dUsed) - pUsed percentage := float64(used) / float64(dTotal) * 100 - threshold := featureflags.BuildCacheMaxUsagePercentage.Fallback() - // When multiple services (template manager, orchestrator) are defined, take the lowest threshold - // to ensure we don't exceed any of the set limits - for _, s := range services { - st := flags.IntFlag(ctx, featureflags.BuildCacheMaxUsagePercentage, featureflags.ServiceContext(string(s))) - if st < threshold { - threshold = st - } - } + threshold := evictionThreshold(ctx, flags, services) if percentage <= float64(threshold) { timer.Reset(getDelay(false)) @@ -252,6 +245,28 @@ func (s *DiffStore) startDiskSpaceEviction( } } +// evictionThreshold returns the maximum allowed disk usage percentage for the +// build cache. When multiple services (template manager, orchestrator) are +// defined, the lowest of their configured thresholds wins to ensure none of +// the set limits is exceeded. Flag evaluation already falls back per service +// inside IntFlag, so the flag fallback is only used directly when no services +// are configured; a flag value above the fallback is honored. +func evictionThreshold(ctx context.Context, flags *featureflags.Client, services cfg.Services) int { + if len(services) == 0 { + return featureflags.BuildCacheMaxUsagePercentage.Fallback() + } + + threshold := math.MaxInt + for _, svc := range services { + st := flags.IntFlag(ctx, featureflags.BuildCacheMaxUsagePercentage, featureflags.ServiceContext(string(svc))) + if st < threshold { + threshold = st + } + } + + return threshold +} + func (s *DiffStore) getPendingDeletesSize() int64 { s.pdMu.RLock() defer s.pdMu.RUnlock() diff --git a/packages/orchestrator/pkg/sandbox/build/cache_test.go b/packages/orchestrator/pkg/sandbox/build/cache_test.go index 2db8f8fc30..ac09acba5d 100644 --- a/packages/orchestrator/pkg/sandbox/build/cache_test.go +++ b/packages/orchestrator/pkg/sandbox/build/cache_test.go @@ -508,6 +508,59 @@ func TestDiffStoreResetDeleteRace(t *testing.T) { time.Sleep(delay * 2) } +func TestEvictionThreshold(t *testing.T) { + t.Parallel() + + t.Run("no services falls back", func(t *testing.T) { + t.Parallel() + + flags := flagsWithMaxBuildCachePercentage(t, 95) + + got := evictionThreshold(t.Context(), flags, nil) + assert.Equal(t, featureflags.BuildCacheMaxUsagePercentage.Fallback(), got) + }) + + t.Run("flag can raise threshold above fallback", func(t *testing.T) { + t.Parallel() + + flags := flagsWithMaxBuildCachePercentage(t, 95) + + got := evictionThreshold(t.Context(), flags, cfg.Services{cfg.Orchestrator}) + assert.Equal(t, 95, got) + }) + + t.Run("flag can lower threshold below fallback", func(t *testing.T) { + t.Parallel() + + flags := flagsWithMaxBuildCachePercentage(t, 10) + + got := evictionThreshold(t.Context(), flags, cfg.Services{cfg.Orchestrator}) + assert.Equal(t, 10, got) + }) + + t.Run("lowest service threshold wins", func(t *testing.T) { + t.Parallel() + + datastore := ldtestdata.DataSource() + datastore.Update( + datastore.Flag(featureflags.BuildCacheMaxUsagePercentage.String()). + Variations(ldvalue.Int(95), ldvalue.Int(40)). + VariationIndexForKey(featureflags.ServiceKind, string(cfg.Orchestrator), 0). + VariationIndexForKey(featureflags.ServiceKind, string(cfg.TemplateManager), 1). + FallthroughVariationIndex(0), + ) + + flags, err := featureflags.NewClientWithDatasource(datastore) + require.NoError(t, err) + t.Cleanup(func() { + assert.NoError(t, flags.Close(t.Context())) + }) + + got := evictionThreshold(t.Context(), flags, cfg.Services{cfg.Orchestrator, cfg.TemplateManager}) + assert.Equal(t, 40, got) + }) +} + func flagsWithMaxBuildCachePercentage(tb testing.TB, maxBuildCachePercentage int) *featureflags.Client { tb.Helper()