From 571d2da4634bd86a4abf7afb1259e920892c4b87 Mon Sep 17 00:00:00 2001 From: Karl Skewes Date: Wed, 2 Sep 2026 13:55:21 +1000 Subject: [PATCH 1/4] feat(ruler): cache parsed rule files in FSLoader Add the experimental -ruler.rule-file-parse-caching-enabled flag. When enabled, FSLoader skips rulefmt.Parse for a rule file whose exact bytes and parse options match the last successful parse, avoiding redundant YAML unmarshalling of unchanged namespace files on every tenant sync. Defaults to disabled. --- pkg/ruler/compat.go | 19 +++- pkg/ruler/mapper.go | 111 ++++++++++++++++++++-- pkg/ruler/mapper_test.go | 199 ++++++++++++++++++++++++++++++++++++++- pkg/ruler/ruler.go | 4 + 4 files changed, 321 insertions(+), 12 deletions(-) diff --git a/pkg/ruler/compat.go b/pkg/ruler/compat.go index 36600ad0631..568754bdbe4 100644 --- a/pkg/ruler/compat.go +++ b/pkg/ruler/compat.go @@ -463,6 +463,18 @@ func DefaultTenantManagerFactory( Help: "Number of queries that did not fetch any series by ruler.", }, []string{"user"}) } + var ruleFileParseCacheHits *prometheus.CounterVec + var ruleFileParseCacheMisses *prometheus.CounterVec + if cfg.RuleFileParseCachingEnabled { + ruleFileParseCacheHits = promauto.With(reg).NewCounterVec(prometheus.CounterOpts{ + Name: "cortex_ruler_rule_file_parse_cache_hits_total", + Help: "Total number of rule file parses served from the parse cache instead of re-parsing.", + }, []string{"user"}) + ruleFileParseCacheMisses = promauto.With(reg).NewCounterVec(prometheus.CounterOpts{ + Name: "cortex_ruler_rule_file_parse_cache_misses_total", + Help: "Total number of rule file parses that were not served from the cache.", + }, []string{"user"}) + } return func(ctx context.Context, userID string, notifier *notifier.Manager, logger log.Logger, reg prometheus.Registerer) RulesManager { var queryTime prometheus.Counter var zeroFetchedSeriesCount prometheus.Counter @@ -470,6 +482,11 @@ func DefaultTenantManagerFactory( queryTime = rulerQuerySeconds.WithLabelValues(userID) zeroFetchedSeriesCount = zeroFetchedSeriesQueries.WithLabelValues(userID) } + var cacheHits, cacheMisses prometheus.Counter + if cfg.RuleFileParseCachingEnabled { + cacheHits = ruleFileParseCacheHits.WithLabelValues(userID) + cacheMisses = ruleFileParseCacheMisses.WithLabelValues(userID) + } // Wrap the query function with our custom logic. wrappedQueryFunc := WrapQueryFuncWithReadConsistency(queryFunc, overrides, userID, logger) remoteQuerier := cfg.QueryFrontend.Address != "" @@ -502,7 +519,7 @@ func DefaultTenantManagerFactory( OutageTolerance: cfg.OutageTolerance, ForGracePeriod: cfg.ForGracePeriod, ResendDelay: cfg.ResendDelay, - GroupLoader: NewFSLoader(rulesFS), + GroupLoader: NewFSLoader(rulesFS, cfg.RuleFileParseCachingEnabled, cacheHits, cacheMisses), RestoreNewRuleGroups: true, DefaultRuleQueryOffset: func() time.Duration { // Delay the evaluation of all rules by a set interval to give a buffer diff --git a/pkg/ruler/mapper.go b/pkg/ruler/mapper.go index 17c1da519d0..aa8723b5fd7 100644 --- a/pkg/ruler/mapper.go +++ b/pkg/ruler/mapper.go @@ -9,14 +9,17 @@ import ( "bytes" "fmt" "log/slog" + "maps" "net/url" "os" "path/filepath" "slices" "strings" + "sync" "github.com/go-kit/log" "github.com/go-kit/log/level" + "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/common/model" "github.com/prometheus/common/promslog" "github.com/prometheus/prometheus/model/rulefmt" @@ -181,42 +184,134 @@ func (m *mapper) writeRuleGroupsIfNewer(groups []rulefmt.RuleGroup, filename str } // FSLoader a GroupLoader implementation that reads files from a given afero.Fs. +// +// If cacheEnabled is set, it caches the parsed result of each file, keyed by +// the file's exact byte content (and the parsing options passed to Load), so +// that a file that hasn't changed since the last Load call skips +// rulefmt.Parse entirely. type FSLoader struct { fs afero.Fs parser parser.Parser logger *slog.Logger + + cacheEnabled bool + cacheHits prometheus.Counter + cacheMisses prometheus.Counter + + mu sync.Mutex + cache map[string]cachedRuleGroups +} + +type cachedRuleGroups struct { + rawBytes []byte + ignoreUnknownFields bool + nameValidationScheme model.ValidationScheme + parsed *rulefmt.RuleGroups } -func NewFSLoader(fs afero.Fs) FSLoader { - return FSLoader{ - fs: fs, - parser: promqlext.NewPromQLParser(), - logger: promslog.NewNopLogger(), +// NewFSLoader returns a GroupLoader that reads rule files from fs. When +// cacheEnabled is true, cacheHits and cacheMisses must be non-nil and are +// incremented on every Load call. +func NewFSLoader(fs afero.Fs, cacheEnabled bool, cacheHits, cacheMisses prometheus.Counter) *FSLoader { + loader := &FSLoader{ + fs: fs, + parser: promqlext.NewPromQLParser(), + logger: promslog.NewNopLogger(), + cacheEnabled: cacheEnabled, + cacheHits: cacheHits, + cacheMisses: cacheMisses, + } + if cacheEnabled { + loader.cache = make(map[string]cachedRuleGroups) } + return loader } -func (f FSLoader) Load(identifier string, ignoreUnknownFields bool, nameValidationScheme model.ValidationScheme) (*rulefmt.RuleGroups, []error) { +func (f *FSLoader) Load(identifier string, ignoreUnknownFields bool, nameValidationScheme model.ValidationScheme) (*rulefmt.RuleGroups, []error) { return f.parseFile(f.fs, identifier, ignoreUnknownFields, nameValidationScheme) } -func (f FSLoader) Parse(query string) (parser.Expr, error) { +func (f *FSLoader) Parse(query string) (parser.Expr, error) { return f.parser.ParseExpr(query) } // parseFile reads and parses rules from a file. // Duplicate of Prometheus' rulefmt.ParseFile, but injects the FS. -func (f FSLoader) parseFile(fs afero.Fs, file string, ignoreUnknownFields bool, nameValidationScheme model.ValidationScheme) (*rulefmt.RuleGroups, []error) { +func (f *FSLoader) parseFile(fs afero.Fs, file string, ignoreUnknownFields bool, nameValidationScheme model.ValidationScheme) (*rulefmt.RuleGroups, []error) { b, err := afero.ReadFile(fs, file) if err != nil { return nil, []error{fmt.Errorf("%s: %w", file, err)} } + + if f.cacheEnabled { + if rgs, ok := f.lookupCache(file, b, ignoreUnknownFields, nameValidationScheme); ok { + f.cacheHits.Inc() + return rgs, nil + } + f.cacheMisses.Inc() + } + rgs, errs := rulefmt.Parse(b, ignoreUnknownFields, nameValidationScheme, f.parser, f.logger) for i := range errs { errs[i] = fmt.Errorf("%s: %w", file, errs[i]) } + if len(errs) == 0 && f.cacheEnabled { + f.storeCache(file, b, ignoreUnknownFields, nameValidationScheme, rgs) + } return rgs, errs } +func (f *FSLoader) lookupCache(path string, rawBytes []byte, ignoreUnknownFields bool, nameValidationScheme model.ValidationScheme) (*rulefmt.RuleGroups, bool) { + f.mu.Lock() + defer f.mu.Unlock() + + entry, ok := f.cache[path] + if !ok || + entry.ignoreUnknownFields != ignoreUnknownFields || + entry.nameValidationScheme != nameValidationScheme || + !bytes.Equal(entry.rawBytes, rawBytes) { + return nil, false + } + return copyRuleGroups(entry.parsed), true +} + +func (f *FSLoader) storeCache(path string, rawBytes []byte, ignoreUnknownFields bool, nameValidationScheme model.ValidationScheme, rgs *rulefmt.RuleGroups) { + f.mu.Lock() + defer f.mu.Unlock() + + f.cache[path] = cachedRuleGroups{ + rawBytes: rawBytes, + ignoreUnknownFields: ignoreUnknownFields, + nameValidationScheme: nameValidationScheme, + parsed: rgs, + } +} + +// copyRuleGroups returns a deep copy of rgs, so that a cache hit never hands +// out a reference into the cached value. This matters because the caller +// (rules.Manager.LoadGroups) stores at least one field, SourceTenants, by +// reference into the resulting rules.Group without copying it, and would +// otherwise alias the cached slice for the lifetime of that group. +func copyRuleGroups(rgs *rulefmt.RuleGroups) *rulefmt.RuleGroups { + if rgs == nil { + return nil + } + out := &rulefmt.RuleGroups{ + Groups: make([]rulefmt.RuleGroup, len(rgs.Groups)), + } + for i, g := range rgs.Groups { + g.SourceTenants = slices.Clone(g.SourceTenants) + g.Rules = make([]rulefmt.Rule, len(rgs.Groups[i].Rules)) + for j, r := range rgs.Groups[i].Rules { + r.Labels = maps.Clone(r.Labels) + r.Annotations = maps.Clone(r.Annotations) + g.Rules[j] = r + } + out.Groups[i] = g + } + return out +} + // cleanRuleGroupExprs returns a copy of groups with leading/trailing whitespace // trimmed from rule expressions. This avoids yaml.v3 emitting explicit // indentation indicators (e.g. "|4") for expressions that start with newlines diff --git a/pkg/ruler/mapper_test.go b/pkg/ruler/mapper_test.go index 9d7196575f1..bab3fde39f3 100644 --- a/pkg/ruler/mapper_test.go +++ b/pkg/ruler/mapper_test.go @@ -6,11 +6,16 @@ package ruler import ( + "fmt" + "io" "net/url" "os" "testing" "github.com/go-kit/log" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/prometheus/client_golang/prometheus/testutil" "github.com/prometheus/common/model" "github.com/prometheus/prometheus/model/rulefmt" "github.com/spf13/afero" @@ -568,7 +573,7 @@ func Test_FSLoader_LoadRules(t *testing.T) { require.Equal(t, fileOneUserOnePath, files[0]) require.NoError(t, err) - loader := NewFSLoader(fs) + loader := NewFSLoader(fs, false, nil, nil) loaded, errs := loader.Load(fileOneUserOnePath, false, model.LegacyValidation) require.Empty(t, errs) require.NotNil(t, loaded) @@ -584,7 +589,7 @@ func Test_FSLoader_LoadRules(t *testing.T) { require.Len(t, files, 2) require.NoError(t, err) - loader := NewFSLoader(fs) + loader := NewFSLoader(fs, false, nil, nil) loaded, errs := loader.Load(fileOneUserOnePath, false, model.LegacyValidation) require.Empty(t, errs) require.NotNil(t, loaded) @@ -604,7 +609,7 @@ func Test_FSLoader_LoadRules(t *testing.T) { require.Len(t, files, 2) require.NoError(t, err) - loader := NewFSLoader(fs) + loader := NewFSLoader(fs, false, nil, nil) loaded, errs := loader.Load(fileOneUserOnePath, false, model.LegacyValidation) require.Empty(t, errs) require.NotNil(t, loaded) @@ -623,6 +628,194 @@ func Test_FSLoader_LoadRules(t *testing.T) { }) } +func newTestCacheCounters() (prometheus.Counter, prometheus.Counter) { + reg := prometheus.NewPedanticRegistry() + hits := promauto.With(reg).NewCounter(prometheus.CounterOpts{Name: "test_rule_file_parse_cache_hits_total"}) + misses := promauto.With(reg).NewCounter(prometheus.CounterOpts{Name: "test_rule_file_parse_cache_misses_total"}) + return hits, misses +} + +func Test_FSLoader_ParseCache(t *testing.T) { + l := util_log.MakeLeveledLogger(os.Stdout, "info") + + t.Run("cache hit returns an equal, distinct result and increments hits", func(t *testing.T) { + setupRuleSets() + fs := afero.NewMemMapFs() + m := &mapper{Path: "/rules", FS: fs, logger: l} + _, files, err := m.MapRules(testUser1, initialRuleSet) + require.NoError(t, err) + + hits, misses := newTestCacheCounters() + loader := NewFSLoader(fs, true, hits, misses) + + first, errs := loader.Load(files[0], false, model.LegacyValidation) + require.Empty(t, errs) + require.Equal(t, 0.0, testutil.ToFloat64(hits)) + require.Equal(t, 1.0, testutil.ToFloat64(misses)) + + second, errs := loader.Load(files[0], false, model.LegacyValidation) + require.Empty(t, errs) + require.Equal(t, 1.0, testutil.ToFloat64(hits)) + require.Equal(t, 1.0, testutil.ToFloat64(misses)) + require.Equal(t, first, second) + require.NotSame(t, first, second, "a cache hit must return a defensive copy, not the cached instance") + }) + + t.Run("cache misses again after the file content changes", func(t *testing.T) { + setupRuleSets() + fs := afero.NewMemMapFs() + m := &mapper{Path: "/rules", FS: fs, logger: l} + _, files, err := m.MapRules(testUser1, initialRuleSet) + require.NoError(t, err) + + hits, misses := newTestCacheCounters() + loader := NewFSLoader(fs, true, hits, misses) + + _, errs := loader.Load(files[0], false, model.LegacyValidation) + require.Empty(t, errs) + + _, _, err = m.MapRules(testUser1, updatedRuleSet) + require.NoError(t, err) + + loaded, errs := loader.Load(files[0], false, model.LegacyValidation) + require.Empty(t, errs) + require.Len(t, loaded.Groups, 3) + require.Equal(t, 0.0, testutil.ToFloat64(hits)) + require.Equal(t, 2.0, testutil.ToFloat64(misses)) + }) + + t.Run("cache is scoped per file path", func(t *testing.T) { + setupRuleSets() + fs := afero.NewMemMapFs() + m := &mapper{Path: "/rules", FS: fs, logger: l} + _, files, err := m.MapRules(testUser1, twoFilesRuleSet) + require.NoError(t, err) + require.Len(t, files, 2) + + hits, misses := newTestCacheCounters() + loader := NewFSLoader(fs, true, hits, misses) + + _, errs := loader.Load(files[0], false, model.LegacyValidation) + require.Empty(t, errs) + _, errs = loader.Load(files[1], false, model.LegacyValidation) + require.Empty(t, errs) + require.Equal(t, 0.0, testutil.ToFloat64(hits)) + require.Equal(t, 2.0, testutil.ToFloat64(misses)) + + _, errs = loader.Load(files[0], false, model.LegacyValidation) + require.Empty(t, errs) + require.Equal(t, 1.0, testutil.ToFloat64(hits)) + require.Equal(t, 2.0, testutil.ToFloat64(misses)) + }) + + t.Run("cache misses when parsing options differ", func(t *testing.T) { + setupRuleSets() + fs := afero.NewMemMapFs() + m := &mapper{Path: "/rules", FS: fs, logger: l} + _, files, err := m.MapRules(testUser1, initialRuleSet) + require.NoError(t, err) + + hits, misses := newTestCacheCounters() + loader := NewFSLoader(fs, true, hits, misses) + + _, errs := loader.Load(files[0], false, model.LegacyValidation) + require.Empty(t, errs) + _, errs = loader.Load(files[0], false, model.UTF8Validation) + require.Empty(t, errs) + require.Equal(t, 0.0, testutil.ToFloat64(hits)) + require.Equal(t, 2.0, testutil.ToFloat64(misses)) + }) + + t.Run("cache disabled behaves like today and never hits", func(t *testing.T) { + setupRuleSets() + fs := afero.NewMemMapFs() + m := &mapper{Path: "/rules", FS: fs, logger: l} + _, files, err := m.MapRules(testUser1, initialRuleSet) + require.NoError(t, err) + + loader := NewFSLoader(fs, false, nil, nil) + first, errs := loader.Load(files[0], false, model.LegacyValidation) + require.Empty(t, errs) + second, errs := loader.Load(files[0], false, model.LegacyValidation) + require.Empty(t, errs) + require.Equal(t, first, second) + require.NotSame(t, first, second) + }) + + t.Run("two loaders don't share cache state", func(t *testing.T) { + setupRuleSets() + fs := afero.NewMemMapFs() + m := &mapper{Path: "/rules", FS: fs, logger: l} + _, files, err := m.MapRules(testUser1, initialRuleSet) + require.NoError(t, err) + + hitsA, missesA := newTestCacheCounters() + loaderA := NewFSLoader(fs, true, hitsA, missesA) + _, errs := loaderA.Load(files[0], false, model.LegacyValidation) + require.Empty(t, errs) + + hitsB, missesB := newTestCacheCounters() + loaderB := NewFSLoader(fs, true, hitsB, missesB) + _, errs = loaderB.Load(files[0], false, model.LegacyValidation) + require.Empty(t, errs) + + require.Equal(t, 0.0, testutil.ToFloat64(hitsB)) + require.Equal(t, 1.0, testutil.ToFloat64(missesB)) + }) +} + +// BenchmarkFSLoader_Load simulates the steady-state case the parse cache +// targets: a tenant's namespace file that hasn't changed being reloaded on +// every rule sync. cache_enabled=false takes the same code path as before +// this cache existed (every Load call re-runs rulefmt.Parse); cache_enabled=true +// exercises the new cache-hit path. +func BenchmarkFSLoader_Load(b *testing.B) { + l := util_log.MakeLeveledLogger(io.Discard, "info") + fs := afero.NewMemMapFs() + m := &mapper{Path: "/rules", FS: fs, logger: l} + + const numGroups = 20 + const rulesPerGroup = 5 + groups := make([]rulefmt.RuleGroup, numGroups) + for i := range groups { + rules := make([]rulefmt.Rule, rulesPerGroup) + for j := range rules { + rules[j] = rulefmt.Rule{ + Record: fmt.Sprintf("rule_%d_%d", i, j), + Expr: fmt.Sprintf("sum(rate(some_metric_%d_%d[5m]))", i, j), + Labels: map[string]string{"team": "observability"}, + } + } + groups[i] = rulefmt.RuleGroup{Name: fmt.Sprintf("group_%d", i), Rules: rules} + } + + _, files, err := m.MapRules("bench_user", map[string][]rulefmt.RuleGroup{"namespace": groups}) + if err != nil { + b.Fatal(err) + } + + for _, cacheEnabled := range []bool{false, true} { + b.Run(fmt.Sprintf("cache_enabled=%v", cacheEnabled), func(b *testing.B) { + var hits, misses prometheus.Counter + if cacheEnabled { + hits, misses = newTestCacheCounters() + } + loader := NewFSLoader(fs, cacheEnabled, hits, misses) + // Warm up so the steady-state (already-parsed-once) case is measured. + if _, errs := loader.Load(files[0], false, model.LegacyValidation); len(errs) > 0 { + b.Fatal(errs) + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + if _, errs := loader.Load(files[0], false, model.LegacyValidation); len(errs) > 0 { + b.Fatal(errs) + } + } + }) + } +} + func requireFileExists(t *testing.T, fs afero.Fs, path string) { t.Helper() diff --git a/pkg/ruler/ruler.go b/pkg/ruler/ruler.go index a86eae47e50..e0453e02412 100644 --- a/pkg/ruler/ruler.go +++ b/pkg/ruler/ruler.go @@ -160,6 +160,8 @@ type Config struct { IndependentRuleEvaluationConcurrencyMinDurationPercentage float64 `yaml:"independent_rule_evaluation_concurrency_min_duration_percentage" category:"experimental"` RuleEvaluationWriteEnabled bool `yaml:"rule_evaluation_write_enabled" category:"experimental"` + + RuleFileParseCachingEnabled bool `yaml:"rule_file_parse_caching_enabled" category:"experimental"` } type ClientConfig struct { @@ -235,6 +237,8 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet, logger log.Logger) { f.BoolVar(&cfg.RuleEvaluationWriteEnabled, "ruler.rule-evaluation-write-enabled", true, "Writes the results of rule evaluation to ingesters or ingest storage when enabled. Use this option for testing purposes. To disable, set to false.") + f.BoolVar(&cfg.RuleFileParseCachingEnabled, "ruler.rule-file-parse-caching-enabled", false, "Cache the result of parsing rule files on disk, keyed by exact file content, to avoid re-parsing unchanged rule files belonging to the same tenant on every rule sync. Internal performance optimization, no effect on rule evaluation behavior.") + f.DurationVar(&cfg.OutboundSyncQueuePollInterval, "ruler.outbound-sync-queue-poll-interval", defaultRulerSyncPollFrequency, `Interval between sending queued rule sync requests to ruler replicas.`) f.DurationVar(&cfg.InboundSyncQueuePollInterval, "ruler.inbound-sync-queue-poll-interval", defaultRulerSyncPollFrequency, `Interval between applying queued incoming rule sync requests.`) From a0597c28198056823d8fe0eeac468e56f6a42de2 Mon Sep 17 00:00:00 2001 From: Karl Skewes Date: Wed, 2 Sep 2026 13:55:21 +1000 Subject: [PATCH 2/4] docs(ruler): document and regenerate reference docs for rule file parse caching flag --- CHANGELOG.md | 1 + cmd/mimir/config-descriptor.json | 11 +++++++++++ cmd/mimir/help-all.txt.tmpl | 2 ++ docs/sources/mimir/configure/about-versioning.md | 2 ++ .../mimir/configure/configuration-parameters/index.md | 7 +++++++ 5 files changed, 23 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50daf9e7992..b3e23d8dfc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ * [ENHANCEMENT] Validation: Add an optional `reason` field to `limited_queries` rules, aligning them with `blocked_queries`. When set, the reason is included in the client-facing error and the query-frontend's `"query limited"` log line. #16407 * [ENHANCEMENT] Compactor: Add the experimental `-compactor.scheduler-client.enable-ring-based-cleanup` option, which when disabled stops a scheduler-mode compactor from running the ring-based background blocks cleaner. #16457 * [ENHANCEMENT] Block-builder-scheduler: Add `cortex_blockbuilder_scheduler_end_offset_probe_failed_total`, counting failures to list a cluster's end offsets, and `cortex_blockbuilder_scheduler_startup_jobs_skipped_total`, counting observed jobs that startup recovery could not import. #16134 +* [ENHANCEMENT] Ruler: Add the experimental `-ruler.rule-file-parse-caching-enabled` option to cache parsed rule files and avoid re-parsing unchanged rule files on every rule sync. #16512 * [FEATURE] Querier: Add experimental per-tenant limit `-querier.max-blocks-per-store-request` to cap the number of blocks a single store-gateway request may reference. Disabled by default. #16292 * [FEATURE] Validation: Add optional `id`, `note`, `created_by`, `created_at`, and `expires_at` fields to `blocked_queries` and `limited_queries` rules, for tooling to attach ownership/context metadata to a rule. For rules with `expires_at` set, the earliest `expires_at` per tenant and `id` (rules without an `id` are grouped together) is exported as the `cortex_blocked_query_rule_expires_at`/`cortex_limited_query_rule_expires_at` metrics, so an alert can fire on stale rules; this is informational only and never affects enforcement. The query-frontend's `"query blocked"` log line now also includes the matched rule's `id` and whether it is expired, and rate-limited queries are now logged with a new `"query limited"` line carrying the same fields. #16395 * [BUGFIX] Query-frontend: Wait for the querier ring to be populated during startup, up to 30 seconds, before reporting the query-frontend as ready. Previously a query-frontend could become ready before it had seen any querier in the ring and fail every query it received until the ring was populated. Only applies when remote execution is enabled, and can be disabled with the experimental `-query-frontend.wait-for-querier-ring-on-startup=false`. #16333 diff --git a/cmd/mimir/config-descriptor.json b/cmd/mimir/config-descriptor.json index 80a64bf3540..8f8e0e32d77 100644 --- a/cmd/mimir/config-descriptor.json +++ b/cmd/mimir/config-descriptor.json @@ -17755,6 +17755,17 @@ "fieldFlag": "ruler.rule-evaluation-write-enabled", "fieldType": "boolean", "fieldCategory": "experimental" + }, + { + "kind": "field", + "name": "rule_file_parse_caching_enabled", + "required": false, + "desc": "Cache the result of parsing rule files on disk, keyed by exact file content, to avoid re-parsing unchanged rule files belonging to the same tenant on every rule sync. Internal performance optimization, no effect on rule evaluation behavior.", + "fieldValue": null, + "fieldDefaultValue": false, + "fieldFlag": "ruler.rule-file-parse-caching-enabled", + "fieldType": "boolean", + "fieldCategory": "experimental" } ], "fieldValue": null, diff --git a/cmd/mimir/help-all.txt.tmpl b/cmd/mimir/help-all.txt.tmpl index 2596ebd5263..f30463746de 100644 --- a/cmd/mimir/help-all.txt.tmpl +++ b/cmd/mimir/help-all.txt.tmpl @@ -3683,6 +3683,8 @@ Usage of ./cmd/mimir/mimir: Backend storage to use for the ring. Supported values are: consul, etcd, inmemory, memberlist, multi. (default "memberlist") -ruler.rule-evaluation-write-enabled [experimental] Writes the results of rule evaluation to ingesters or ingest storage when enabled. Use this option for testing purposes. To disable, set to false. (default true) + -ruler.rule-file-parse-caching-enabled + [experimental] Cache the result of parsing rule files on disk, keyed by exact file content, to avoid re-parsing unchanged rule files belonging to the same tenant on every rule sync. Internal performance optimization, no effect on rule evaluation behavior. -ruler.rule-path string Directory to store temporary rule files loaded by the Prometheus rule managers. This directory is not required to be persisted between restarts. (default "./data-ruler/") -ruler.sync-rules-on-changes-enabled diff --git a/docs/sources/mimir/configure/about-versioning.md b/docs/sources/mimir/configure/about-versioning.md index 5dc704e18bc..a29e8f36825 100644 --- a/docs/sources/mimir/configure/about-versioning.md +++ b/docs/sources/mimir/configure/about-versioning.md @@ -83,6 +83,8 @@ The following features are currently experimental: - `-ruler.max-independent-rule-evaluation-concurrency-per-tenant` - `-ruler.independent-rule-evaluation-concurrency-min-duration-percentage` - `-ruler.rule-evaluation-write-enabled` + - Cache the result of parsing rule files on disk to avoid re-parsing unchanged rule files on every rule sync. + - `-ruler.rule-file-parse-caching-enabled` - Push rule-result series to remote distributors over native gRPC instead of using the internal distributor. - `-ruler.distributor.address` - `-ruler.distributor.remote-timeout` diff --git a/docs/sources/mimir/configure/configuration-parameters/index.md b/docs/sources/mimir/configure/configuration-parameters/index.md index e13c1f685e3..7b5dbe1de53 100644 --- a/docs/sources/mimir/configure/configuration-parameters/index.md +++ b/docs/sources/mimir/configure/configuration-parameters/index.md @@ -3134,6 +3134,13 @@ tenant_federation: # false. # CLI flag: -ruler.rule-evaluation-write-enabled [rule_evaluation_write_enabled: | default = true] + +# (experimental) Cache the result of parsing rule files on disk, keyed by exact +# file content, to avoid re-parsing unchanged rule files belonging to the same +# tenant on every rule sync. Internal performance optimization, no effect on +# rule evaluation behavior. +# CLI flag: -ruler.rule-file-parse-caching-enabled +[rule_file_parse_caching_enabled: | default = false] ``` ### ruler_storage From 414c0ea85802056475807798cb72ad2b1c4395fb Mon Sep 17 00:00:00 2001 From: Karl Skewes Date: Wed, 2 Sep 2026 14:36:00 +1000 Subject: [PATCH 3/4] fix(ruler): copy rule groups before caching to avoid aliasing source tenants A cache miss stored and returned the same *rulefmt.RuleGroups pointer. rules.Manager.LoadGroups keeps SourceTenants aliased by reference, and federated rule evaluation later sorts that slice in place (tenant.NormalizeTenantIDs), racing with any later cache read of the same entry. Cache a defensive copy instead. Found by Cursor Bugbot: https://github.com/grafana/mimir/pull/16512#discussion_r3910715547 --- pkg/ruler/mapper.go | 9 +++---- pkg/ruler/mapper_test.go | 54 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/pkg/ruler/mapper.go b/pkg/ruler/mapper.go index aa8723b5fd7..9e0de609308 100644 --- a/pkg/ruler/mapper.go +++ b/pkg/ruler/mapper.go @@ -256,7 +256,8 @@ func (f *FSLoader) parseFile(fs afero.Fs, file string, ignoreUnknownFields bool, errs[i] = fmt.Errorf("%s: %w", file, errs[i]) } if len(errs) == 0 && f.cacheEnabled { - f.storeCache(file, b, ignoreUnknownFields, nameValidationScheme, rgs) + // Cache a copy, not rgs itself: its SourceTenants field is mutated in place by the caller. + f.storeCache(file, b, ignoreUnknownFields, nameValidationScheme, copyRuleGroups(rgs)) } return rgs, errs } @@ -287,11 +288,7 @@ func (f *FSLoader) storeCache(path string, rawBytes []byte, ignoreUnknownFields } } -// copyRuleGroups returns a deep copy of rgs, so that a cache hit never hands -// out a reference into the cached value. This matters because the caller -// (rules.Manager.LoadGroups) stores at least one field, SourceTenants, by -// reference into the resulting rules.Group without copying it, and would -// otherwise alias the cached slice for the lifetime of that group. +// copyRuleGroups returns a deep copy of rgs. func copyRuleGroups(rgs *rulefmt.RuleGroups) *rulefmt.RuleGroups { if rgs == nil { return nil diff --git a/pkg/ruler/mapper_test.go b/pkg/ruler/mapper_test.go index bab3fde39f3..c0d6be390ab 100644 --- a/pkg/ruler/mapper_test.go +++ b/pkg/ruler/mapper_test.go @@ -10,6 +10,7 @@ import ( "io" "net/url" "os" + "slices" "testing" "github.com/go-kit/log" @@ -673,6 +674,8 @@ func Test_FSLoader_ParseCache(t *testing.T) { _, errs := loader.Load(files[0], false, model.LegacyValidation) require.Empty(t, errs) + require.Equal(t, 0.0, testutil.ToFloat64(hits)) + require.Equal(t, 1.0, testutil.ToFloat64(misses)) _, _, err = m.MapRules(testUser1, updatedRuleSet) require.NoError(t, err) @@ -697,6 +700,9 @@ func Test_FSLoader_ParseCache(t *testing.T) { _, errs := loader.Load(files[0], false, model.LegacyValidation) require.Empty(t, errs) + require.Equal(t, 0.0, testutil.ToFloat64(hits)) + require.Equal(t, 1.0, testutil.ToFloat64(misses)) + _, errs = loader.Load(files[1], false, model.LegacyValidation) require.Empty(t, errs) require.Equal(t, 0.0, testutil.ToFloat64(hits)) @@ -720,6 +726,9 @@ func Test_FSLoader_ParseCache(t *testing.T) { _, errs := loader.Load(files[0], false, model.LegacyValidation) require.Empty(t, errs) + require.Equal(t, 0.0, testutil.ToFloat64(hits)) + require.Equal(t, 1.0, testutil.ToFloat64(misses)) + _, errs = loader.Load(files[0], false, model.UTF8Validation) require.Empty(t, errs) require.Equal(t, 0.0, testutil.ToFloat64(hits)) @@ -742,6 +751,49 @@ func Test_FSLoader_ParseCache(t *testing.T) { require.NotSame(t, first, second) }) + t.Run("mutating a cache-miss result doesn't corrupt a later cache hit", func(t *testing.T) { + // Regression test: LoadGroups keeps SourceTenants aliased by reference + // (it isn't copied like Labels/Annotations are), and federated rule + // evaluation later sorts that slice in place. The cache must never + // hand out a value that shares memory with what it stores internally. + setupRuleSets() + fs := afero.NewMemMapFs() + m := &mapper{Path: "/rules", FS: fs, logger: l} + ruleConfigs := map[string][]rulefmt.RuleGroup{ + "file /one": { + { + Name: "federated_group", + SourceTenants: []string{"tenant-b", "tenant-a"}, + Rules: []rulefmt.Rule{ + {Record: "example_rule", Expr: "example_expr"}, + }, + }, + }, + } + _, files, err := m.MapRules(testUser1, ruleConfigs) + require.NoError(t, err) + + hits, misses := newTestCacheCounters() + loader := NewFSLoader(fs, true, hits, misses) + + missResult, errs := loader.Load(files[0], false, model.LegacyValidation) + require.Empty(t, errs) + require.Equal(t, 0.0, testutil.ToFloat64(hits)) + require.Equal(t, 1.0, testutil.ToFloat64(misses)) + + // Simulate what tenant.NormalizeTenantIDs does in place during + // federated rule evaluation on the caller's copy of SourceTenants. + slices.Sort(missResult.Groups[0].SourceTenants) + require.Equal(t, []string{"tenant-a", "tenant-b"}, missResult.Groups[0].SourceTenants) + + hitResult, errs := loader.Load(files[0], false, model.LegacyValidation) + require.Empty(t, errs) + require.Equal(t, 1.0, testutil.ToFloat64(hits)) + require.Equal(t, 1.0, testutil.ToFloat64(misses)) + require.Equal(t, []string{"tenant-b", "tenant-a"}, hitResult.Groups[0].SourceTenants, + "the cached entry must be unaffected by mutating a previously returned result") + }) + t.Run("two loaders don't share cache state", func(t *testing.T) { setupRuleSets() fs := afero.NewMemMapFs() @@ -753,6 +805,8 @@ func Test_FSLoader_ParseCache(t *testing.T) { loaderA := NewFSLoader(fs, true, hitsA, missesA) _, errs := loaderA.Load(files[0], false, model.LegacyValidation) require.Empty(t, errs) + require.Equal(t, 0.0, testutil.ToFloat64(hitsA)) + require.Equal(t, 1.0, testutil.ToFloat64(missesA)) hitsB, missesB := newTestCacheCounters() loaderB := NewFSLoader(fs, true, hitsB, missesB) From bacb53aafea5362e5bee877e8d0fe8e4df44c8f9 Mon Sep 17 00:00:00 2001 From: Karl Skewes Date: Thu, 3 Sep 2026 11:06:17 +1000 Subject: [PATCH 4/4] fix(ruler): bound FSLoader cache growth with generational eviction A namespace file's cache entry was never removed on rename or delete. It was only freed when the tenant's whole FSLoader was torn down. A tenant with ongoing namespace churn would accumulate one dead entry per distinct path ever seen. That growth was unbounded for the life of the tenant's manager assignment. Split the cache into two generations, cur and prev. rules.Manager calls Load at most once per path per Update pass. Seeing a path already in cur means a new pass started, so rotate. A prev hit is promoted back into cur. This keeps a stable file triggering rotation on later passes. A path that stops being loaded ages out within two passes, once prev is next overwritten. This bounds the cache at roughly 2x the live file count, regardless of total distinct paths ever used. Known gap: a tenant whose entire namespace set changes to new paths on every single pass, with no namespace ever stable across two consecutive passes, never triggers a rotation. This defeats the bound. Considered acceptable. It requires zero stable namespaces ever, not just occasional renames. --- pkg/ruler/mapper.go | 44 ++++++++++++++++++++++++++++-------- pkg/ruler/mapper_test.go | 49 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/pkg/ruler/mapper.go b/pkg/ruler/mapper.go index 9e0de609308..1dcb95f5a0a 100644 --- a/pkg/ruler/mapper.go +++ b/pkg/ruler/mapper.go @@ -185,10 +185,29 @@ func (m *mapper) writeRuleGroupsIfNewer(groups []rulefmt.RuleGroup, filename str // FSLoader a GroupLoader implementation that reads files from a given afero.Fs. // -// If cacheEnabled is set, it caches the parsed result of each file, keyed by -// the file's exact byte content (and the parsing options passed to Load), so -// that a file that hasn't changed since the last Load call skips -// rulefmt.Parse entirely. +// If cacheEnabled is set, it caches the parsed result of each file. +// The cache key is the file's exact byte content plus the parsing options passed to Load. +// A file that hasn't changed since the last Load call skips rulefmt.Parse entirely. +// +// The cache holds two generations, cur and prev, to bound memory. +// This avoids needing an external signal for when a file is removed or renamed. +// rules.Manager.LoadGroups calls Load at most once per path per Update pass. +// manager.Update holds its own lock. Passes for one tenant never interleave. +// A path already present in cur means a new pass has started. +// Load then rotates the generations: prev = cur, cur = a new empty map. +// +// A hit in prev gets promoted into cur. +// This keeps a stable file triggering rotation on every later pass. +// Without promotion, a stable file would fall out of the cache after a single pass. +// +// A path that stops being loaded ages out within two passes. +// Its entry moves into prev on the next rotation. +// It is dropped for good when prev is next overwritten. +// This bounds the cache at roughly 2x the live file count, regardless of how many distinct paths a tenant has ever used. +// +// Known gap: this needs at least one stable path across two consecutive passes to trigger a rotation. +// A tenant whose entire namespace set changes to new paths on every single pass never triggers one. +// That is considered acceptable. It requires zero stable namespaces ever, not just occasional renames. type FSLoader struct { fs afero.Fs parser parser.Parser @@ -198,8 +217,8 @@ type FSLoader struct { cacheHits prometheus.Counter cacheMisses prometheus.Counter - mu sync.Mutex - cache map[string]cachedRuleGroups + mu sync.Mutex + cur, prev map[string]cachedRuleGroups } type cachedRuleGroups struct { @@ -222,7 +241,7 @@ func NewFSLoader(fs afero.Fs, cacheEnabled bool, cacheHits, cacheMisses promethe cacheMisses: cacheMisses, } if cacheEnabled { - loader.cache = make(map[string]cachedRuleGroups) + loader.cur = make(map[string]cachedRuleGroups) } return loader } @@ -266,13 +285,20 @@ func (f *FSLoader) lookupCache(path string, rawBytes []byte, ignoreUnknownFields f.mu.Lock() defer f.mu.Unlock() - entry, ok := f.cache[path] + if _, ok := f.cur[path]; ok { + f.prev = f.cur + f.cur = make(map[string]cachedRuleGroups) + } + + entry, ok := f.prev[path] if !ok || entry.ignoreUnknownFields != ignoreUnknownFields || entry.nameValidationScheme != nameValidationScheme || !bytes.Equal(entry.rawBytes, rawBytes) { return nil, false } + + f.cur[path] = entry return copyRuleGroups(entry.parsed), true } @@ -280,7 +306,7 @@ func (f *FSLoader) storeCache(path string, rawBytes []byte, ignoreUnknownFields f.mu.Lock() defer f.mu.Unlock() - f.cache[path] = cachedRuleGroups{ + f.cur[path] = cachedRuleGroups{ rawBytes: rawBytes, ignoreUnknownFields: ignoreUnknownFields, nameValidationScheme: nameValidationScheme, diff --git a/pkg/ruler/mapper_test.go b/pkg/ruler/mapper_test.go index c0d6be390ab..db614fb0e38 100644 --- a/pkg/ruler/mapper_test.go +++ b/pkg/ruler/mapper_test.go @@ -870,6 +870,55 @@ func BenchmarkFSLoader_Load(b *testing.B) { } } +func Test_FSLoader_ParseCache_EvictsRemovedNamespaces(t *testing.T) { + // Regression test for unbounded growth: a removed or renamed namespace's + // cache entry must not live forever. It should age out within two more + // passes of the tenant's remaining (stable) namespace, via generation + // rotation, not accumulate for as long as the FSLoader exists. + l := util_log.MakeLeveledLogger(os.Stdout, "info") + setupRuleSets() + fs := afero.NewMemMapFs() + m := &mapper{Path: "/rules", FS: fs, logger: l} + + hits, misses := newTestCacheCounters() + loader := NewFSLoader(fs, true, hits, misses) + + loadAll := func(files []string) { + for _, f := range files { + _, errs := loader.Load(f, false, model.LegacyValidation) + require.Empty(t, errs) + } + } + + // Pass 1: two namespaces. + _, files, err := m.MapRules(testUser1, twoFilesRuleSet) + require.NoError(t, err) + require.Len(t, files, 2) + loadAll(files) + require.Len(t, loader.cur, 2) + require.Empty(t, loader.prev) + + // Pass 2: same two namespaces, unchanged -- triggers the first rotation. + loadAll(files) + require.Len(t, loader.cur, 2) + require.Len(t, loader.prev, 2) + + // Pass 3: one namespace is removed. The survivor's rotation moves the + // removed namespace's stale entry into prev, where it lingers once more. + _, remainingFiles, err := m.MapRules(testUser1, twoFilesDeletedRuleSet) + require.NoError(t, err) + require.Len(t, remainingFiles, 1) + loadAll(remainingFiles) + require.Len(t, loader.cur, 1) + require.Len(t, loader.prev, 2, "the removed namespace's entry should still be in prev for one more pass") + + // Pass 4: the next rotation overwrites prev, dropping the removed + // namespace's entry for good. + loadAll(remainingFiles) + require.Len(t, loader.cur, 1) + require.Len(t, loader.prev, 1, "the removed namespace's entry must be gone after a second rotation") +} + func requireFileExists(t *testing.T, fs afero.Fs, path string) { t.Helper()