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 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..1dcb95f5a0a 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,157 @@ 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. +// 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 logger *slog.Logger + + cacheEnabled bool + cacheHits prometheus.Counter + cacheMisses prometheus.Counter + + mu sync.Mutex + cur, prev 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.cur = 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 { + // 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 } +func (f *FSLoader) lookupCache(path string, rawBytes []byte, ignoreUnknownFields bool, nameValidationScheme model.ValidationScheme) (*rulefmt.RuleGroups, bool) { + f.mu.Lock() + defer f.mu.Unlock() + + 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 +} + +func (f *FSLoader) storeCache(path string, rawBytes []byte, ignoreUnknownFields bool, nameValidationScheme model.ValidationScheme, rgs *rulefmt.RuleGroups) { + f.mu.Lock() + defer f.mu.Unlock() + + f.cur[path] = cachedRuleGroups{ + rawBytes: rawBytes, + ignoreUnknownFields: ignoreUnknownFields, + nameValidationScheme: nameValidationScheme, + parsed: rgs, + } +} + +// copyRuleGroups returns a deep copy of rgs. +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..db614fb0e38 100644 --- a/pkg/ruler/mapper_test.go +++ b/pkg/ruler/mapper_test.go @@ -6,11 +6,17 @@ package ruler import ( + "fmt" + "io" "net/url" "os" + "slices" "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 +574,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 +590,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 +610,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 +629,296 @@ 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) + 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) + + 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) + 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)) + 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) + 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)) + 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("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() + 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) + 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) + _, 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 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() 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.`)