abbf05ac29ca2d9825652b575c0b611b4e353b51
1
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Sharon KatzandClaude Opus 4.6
|
74b10d8a0d |
Cache pack config JSON per team to reduce redundant marshaling (#48702)
**Related issue:** #21847 ## Summary `GetClientConfig` is called by every host every ~60 seconds. It rebuilds the full pack config (all scheduled query SQL text) from DB and JSON-marshals it on every request. For all hosts in the same team, the result is identical, yet we run 3-5 DB queries + `json.Marshal` of ~50KB per request. This PR adds an in-memory cache for the marshaled pack config JSON, keyed by `(teamID, queryReportsDisabled)` with a 1-minute TTL. The cache is invalidated when queries or AppConfig are modified. ### What changed - Extracted pack config building from `GetClientConfig` into a new `getPackConfig` method - Added `packConfigCache` field to Service struct using `go-cache` (1-minute TTL, 5-minute cleanup) - On cache hit (no legacy packs): returns cached `json.RawMessage` immediately, skipping all DB queries and JSON marshaling - On cache miss: builds pack config from DB, marshals, caches, and returns - Cache is flushed on any query mutation (`NewQuery`, `ModifyQuery`, `DeleteQuery`, `DeleteQueries`, `ApplyQuerySpecs`, `DeleteQueryByID`) and on `ModifyAppConfig` ### Expected impact at 100K hosts | Metric | Before | After | |--------|--------|-------| | Pack config marshals/second | ~1,667 | ~1 per minute per team | | DB queries for scheduled queries/second | ~5,000 | ~5 per minute per team | | CPU from JSON encoding | Dominant in pprof | Negligible | ### Known limitation `ListScheduledQueriesForAgents` supports label-scoped query filtering per host. The cache is keyed by team (not host), so when label-scoped scheduled queries exist, all hosts in a team receive the same query set from the cache regardless of their label memberships. This is an acceptable trade-off because: - Label-scoped scheduled queries are uncommon in most deployments - The cache TTL is 1 minute, so divergence is temporary - Running an extra query on a host is not harmful (just unnecessary work) - This can be refined in a follow-up to filter label-scoped queries from the cached result ## Testing ### Unit tests (9 tests, all pass) | Test | What it verifies | |------|-----------------| | `TestPackConfigCacheHit` | Second `GetClientConfig` call triggers zero DB calls for scheduled queries | | `TestPackConfigCacheInvalidationOnQueryCreate` | After `InvalidatePackConfigCache()`, new query appears in config | | `TestPackConfigCacheInvalidationOnQueryModify` | After invalidation, updated SQL is reflected in config | | `TestPackConfigCacheInvalidationOnQueryDelete` | After invalidation with empty query list, packs key is absent | | `TestPackConfigCacheInvalidationOnApplyQuerySpecs` | After invalidation simulating GitOps apply, new specs appear | | `TestPackConfigCacheTTLExpiration` | After 50ms TTL expires, fresh DB read occurs and new query appears | | `TestPackConfigCacheTeamIsolation` | Global, team-1, team-2 hosts get correctly isolated cached configs | | `TestPackConfigCacheLegacyPacksBypass` | Host with legacy pack triggers DB calls on every request (no caching) | | `TestPackConfigCachePerformance` | 1000 cached calls: 0 DB calls. 1000 uncached: 1000 DB calls. ~1.4x speedup with mock (real DB would be much larger) | ``` === RUN TestPackConfigCacheHit --- PASS (0.01s) === RUN TestPackConfigCacheInvalidationOnQueryCreate --- PASS (0.01s) === RUN TestPackConfigCacheInvalidationOnQueryModify --- PASS (0.01s) === RUN TestPackConfigCacheInvalidationOnQueryDelete --- PASS (0.01s) === RUN TestPackConfigCacheInvalidationOnApplyQuerySpecs --- PASS (0.01s) === RUN TestPackConfigCacheTTLExpiration --- PASS (0.11s) === RUN TestPackConfigCacheTeamIsolation --- PASS (0.01s) === RUN TestPackConfigCacheLegacyPacksBypass --- PASS (0.01s) === RUN TestPackConfigCachePerformance --- PASS (0.02s) Performance: cached=2.37ms, uncached=3.42ms, speedup=1.4x ``` Note: The 1.4x speedup is with mock datastore (no real DB/network). With real MySQL over network, the speedup would be orders of magnitude larger since cached calls skip 3-5 DB round-trips + ~50KB JSON marshal entirely. # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually - [x] Confirmed that the fix is not expected to adversely impact load test results ## QA: Load test verification To validate the real-world impact, QA should run a load test before and after this change and compare: 1. Capture a CPU pprof profile **before** the change under load (e.g., 10K+ simulated hosts, 50+ scheduled queries) 2. Deploy the change and capture a **second** pprof profile under the same load 3. Compare the flamegraphs -- the `encoding/json.Marshal` and `GetClientConfig` CPU time should drop significantly 4. Monitor Fleet container CPU utilization -- expect a measurable reduction in steady-state CPU See #21847 for the original pprof showing `encoding/json` dominating CPU at scale. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved host config response performance by caching pack configuration data. * Query changes now automatically refresh cached host config so updates appear promptly. * **Bug Fixes** * Host configs now stay accurate after creating, updating, deleting, or applying queries. * Cached data is isolated correctly and falls back to fresh data when legacy packs are present. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |