Improved the performance of the configuration profiles status summary - #48873
Conversation
|
@coderabbitai full review |
|
/agentic_review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
This PR addresses timeouts in GET /configuration_profiles/summary for Windows at large scale by introducing a per-host rollup table (host_mdm_windows_profiles_status) that materializes each host’s aggregate profiles delivery status, and updating the summary query to read that rollup instead of performing an O(hosts × profiles) correlated aggregation.
Changes:
- Add
host_mdm_windows_profiles_statustable + migration backfill to store one status bucket per Windows host. - Maintain the rollup incrementally across Windows profile write paths, and add an hourly reconcile job as a self-healing sweep.
- Update/extend tests to validate rollup correctness and to reconcile before reading summaries in tests that seed profile rows directly.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| changes/48340-windows-config-profiles-summary-scale | Release note / changes entry (content excluded by policy). |
| server/service/integration_mdm_test.go | Reconcile rollup before asserting Windows profile summaries in integration tests. |
| server/mock/datastore_mock.go | Add mock hook for ReconcileWindowsProfilesStatus. |
| server/fleet/datastore.go | Extend datastore interface with ReconcileWindowsProfilesStatus. |
| server/datastore/mysql/schema.sql | Add host_mdm_windows_profiles_status table to schema. |
| server/datastore/mysql/migrations/tables/20260707150000_AddHostMDMWindowsProfilesStatus.go | Migration to create/backfill the rollup table. |
| server/datastore/mysql/migrations/tables/20260707150000_AddHostMDMWindowsProfilesStatus_test.go | Migration test for rollup backfill logic. |
| server/datastore/mysql/microsoft_mdm.go | Rollup maintenance helper, reconcile implementation, and summary query changes to read rollup. |
| server/datastore/mysql/microsoft_mdm_test.go | Add/adjust tests to validate rollup maintenance and reconcile behavior. |
| server/datastore/mysql/mdm.go | Ensure Windows MDM cleanup also clears the new rollup table. |
| server/datastore/mysql/mdm_test.go | Reconcile rollup before Windows summary assertions in tests that seed rows directly. |
| server/datastore/mysql/hosts.go | Ensure host deletion cleanup includes host_mdm_windows_profiles_status. |
| cmd/fleet/cron.go | Add hourly cleanup job to reconcile Windows profiles status rollup. |
Files excluded by content exclusion policy (1)
- changes/48340-windows-config-profiles-summary-scale
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
WalkthroughAdds a per-host Windows MDM profile status rollup table, updates Windows profile write and cleanup paths to maintain it, and introduces paged reconciliation for drift and orphan rows. Windows profile summary queries now read the rollup instead of recomputing per-host aggregates. Reconciliation is exposed through datastore interfaces and mocks, scheduled hourly, and covered by datastore, integration, and host-deletion tests. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
…s-status-rollup # Conflicts: # server/datastore/mysql/schema.sql
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #48873 +/- ##
==========================================
+ Coverage 67.89% 67.91% +0.02%
==========================================
Files 3890 3891 +1
Lines 248429 249026 +597
Branches 13179 13179
==========================================
+ Hits 168676 169136 +460
- Misses 64540 64623 +83
- Partials 15213 15267 +54
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…s-status-rollup # Conflicts: # server/datastore/mysql/schema.sql
|
@coderabbitai full review |
|
/agentic_review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 4 comments.
Files excluded by content exclusion policy (1)
- changes/48340-windows-config-profiles-summary-scale
|
Code review by qodo was updated up to the latest commit e899bbb |
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Files excluded by content exclusion policy (1)
- changes/48340-windows-config-profiles-summary-scale
Comments suppressed due to low confidence (1)
server/datastore/mysql/mdm.go:2422
- The rollup refresh selects all Windows hosts where this profile now has
status IS NULL, which can include hosts that were already pending before this resend. At scale, that can cause an unnecessarily large rollup recompute. Capture the affected host UUIDs using the same status filter as the UPDATE (ideallyFOR UPDATE) and refresh only those hosts.
// Refresh the per-host Windows profile status rollup for the affected hosts in the same transaction.
if table == "host_mdm_windows_profiles" {
var windowsHostUUIDs []string
if err := sqlx.SelectContext(ctx, tx, &windowsHostUUIDs,
`SELECT host_uuid FROM host_mdm_windows_profiles WHERE profile_uuid = ? AND status IS NULL`,
profileUUID); err != nil {
return ctxerr.Wrap(ctx, err, "selecting affected hosts for batch resend")
}
…s-status-rollup # Conflicts: # server/datastore/mysql/schema.sql
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Files excluded by content exclusion policy (1)
- changes/48340-windows-config-profiles-summary-scale
Comments suppressed due to low confidence (1)
cmd/fleet/cron.go:1463
- The cleanups/aggregation cron runs on a 1h interval (defaultInterval in newCleanupsAndAggregationSchedule). Running ReconcileWindowsProfilesStatus on every tick will page through all hosts/profiles and can reintroduce significant read load at scale (essentially scanning host_mdm_windows_profiles regularly), which undermines the performance goal of moving /configuration_profiles/summary to an O(hosts) read.
Consider moving this reconciler to its own schedule with a much longer interval (e.g. daily) and/or gating it behind a config flag so large deployments aren't forced into an hourly full-table reconcile.
schedule.WithJob(
// Self-healing safety net for the per-host Windows profile status rollup. This reconciles any drift and removes orphan rows.
"windows_profiles_status_reconcile",
func(ctx context.Context) error {
return ds.ReconcileWindowsProfilesStatus(ctx)
},
|
@coderabbitai review |
|
/agentic_review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated no new comments.
Files excluded by content exclusion policy (1)
- changes/48340-windows-config-profiles-summary-scale
Comments suppressed due to low confidence (1)
server/datastore/mysql/mdm.go:2431
BatchResendMDMProfileToHostsnow runs an unboundedSELECT host_uuid ...inside the transaction to collect every affected Windows host for the async rollup refresh. On large fleets this can (a) double-scan the same large set of rows right after the UPDATE, (b) hold the transaction open while streaming a potentially huge result set, and (c) allocate a very large slice in memory.
Consider paging host UUIDs in batches (e.g., WHERE profile_uuid = ? AND status IS NULL AND host_uuid > ? ORDER BY host_uuid LIMIT ?) and dispatching rollup refresh per page (or performing the rollup recompute directly in a bounded background loop) to keep memory and transaction time bounded.
// Collect the affected hosts for the rollup refresh. Selecting status IS NULL rows AFTER the update sees this transaction's own
// writes, so it cannot miss a row the update touched; rows already NULL are harmless extras (the recompute is idempotent). The
// refresh itself is dispatched asynchronously after commit: the affected set scales with the fleet (a fleet-wide failed-profile
// resend touches every host), and a crash before it completes is healed by the hourly reconcile.
if table == "host_mdm_windows_profiles" {
if err := sqlx.SelectContext(ctx, tx, &windowsHostUUIDs,
`SELECT host_uuid FROM host_mdm_windows_profiles WHERE profile_uuid = ? AND status IS NULL`,
profileUUID); err != nil {
return ctxerr.Wrap(ctx, err, "selecting affected hosts for batch resend")
}
| status VARCHAR(20) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '', | ||
| updated_at DATETIME(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), | ||
| PRIMARY KEY (host_uuid), | ||
| KEY idx_host_mdm_windows_profiles_status_status (status) |
There was a problem hiding this comment.
I don't see this index being used anywhere?
There was a problem hiding this comment.
@juan-fdz-hawa You're right. It is for the follow-up work to add this to the Hosts page os_settings filter (os_settings=pending). Let's keep it here so we don't need another migration in the follow-up PR.
Related issue: Resolves #48340
Windows only. The fix is to use a rollup status table instead of recalculating the host profile summary on demand.
Verified the fix in load test with 100k Windows MDM hosts. Note that this does not improve the host details page filtered by OS settings, which will be handled by the follow up #48996
Checklist for submitter
If some of the following don't apply, delete the relevant line.
Changes file added for user-visible changes in
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Timeouts are implemented and retries are limited to avoid infinite loops
Testing
Added/updated automated tests
Where appropriate, automated tests simulate multiple hosts and test for host isolation (updates to one hosts's records do not affect another)
QA'd all new/changed functionality manually
Database migrations
COLLATE utf8mb4_unicode_ci).Summary by CodeRabbit