MDM Windows push perf fixes - #46917
Merged
Merged
Conversation
Relax the Windows MDM DMClient poll schedule in-session, gated on the host's fleetd (orbit) version and reconciled against a persisted applied-state column. - getPollScheduleCommands: desired-relaxed (orbit >= 1.56.0, via GetHostOrbitInfo) vs applied (mdm_windows_enrollments.poll_schedule_relaxed). Emits one Replace on .../Poll/IntervalForFirstSetOfRetries (60 relax / 1 restore) only when they differ. - windowsMDMHostSupportsSync: missing/unlinked host or unknown version => not-capable (stay on the fast poll, the safe default). - Migration 20260601204606 adds poll_schedule_relaxed TINYINT(1) NOT NULL DEFAULT 0. - Datastore setter + enrolled-device SELECT + interface + regenerated mock. - Tests: TestGetPollScheduleCommands, testMDMWindowsPollScheduleRelaxed, migration test.
…dm-wake # Conflicts: # server/datastore/mysql/schema.sql # server/service/microsoft_mdm_test.go
…edule setter; tidy tests
…dm-wake # Conflicts: # server/datastore/mysql/schema.sql
# Conflicts: # server/datastore/mysql/microsoft_mdm.go # server/datastore/mysql/microsoft_mdm_test.go # server/datastore/mysql/schema.sql
The merge from main reintroduced the old recompute-on-ack assertion above the updated soft-dequeue + per-session refresh assertions.
Member
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Member
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Member
Author
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 2eecab2 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/datastore/mysql/microsoft_mdm.go`:
- Around line 349-357: The recompute currently triggered by
MDMWindowsRefreshHasPendingCommands (which calls
recomputeMDMWindowsHasPendingCommandsByEnrollmentIDs) should run only when there
are no non-poll pending commands left, not when the full pending-command set is
empty; update the callers that currently run the recompute after
MDMWindowsGetPendingCommands returns zero rows so they instead inspect the
returned rows and trigger recompute only if none of the rows are a poll-schedule
Replace (i.e., filter out poll-schedule/poll commands by their command
type/target and treat those as excluded), and apply the same change to the other
call sites that use MDMWindowsGetPendingCommands before calling
recomputeMDMWindowsHasPendingCommandsByEnrollmentIDs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bad51649-b612-46d7-a33b-bfd05b1b9f44
📒 Files selected for processing (11)
server/datastore/mysql/mdm.goserver/datastore/mysql/microsoft_mdm.goserver/datastore/mysql/microsoft_mdm_test.goserver/datastore/mysql/migrations/tables/20260605151542_AddAckedAtToWindowsMDMCommandQueue.goserver/datastore/mysql/migrations/tables/20260605151542_AddAckedAtToWindowsMDMCommandQueue_test.goserver/datastore/mysql/schema.sqlserver/fleet/datastore.goserver/fleet/microsoft_mdm.goserver/mock/datastore_mock.goserver/service/microsoft_mdm.goserver/service/microsoft_mdm_test.go
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #46917 +/- ##
==========================================
- Coverage 67.04% 67.03% -0.01%
==========================================
Files 2867 2864 -3
Lines 225117 225059 -58
Branches 11743 11578 -165
==========================================
- Hits 150926 150869 -57
+ Misses 60521 60515 -6
- Partials 13670 13675 +5
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:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
ksykulev
previously approved these changes
Jun 5, 2026
ksykulev
left a comment
ksykulev
left a comment
Contributor
There was a problem hiding this comment.
Looks good to me.
Just need to merge main and re-number the migration and it's good to go.
# Conflicts: # server/datastore/mysql/schema.sql
Member
Author
|
@ksykulev merged with main. Ready for re-approval. |
ksykulev
approved these changes
Jun 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue: Resolves #46567
Loadtest feedback: reducing the number of
UPDATE mdm_windows_enrollments e SET e.has_pending_commandswrites.Checklist for submitter
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
Summary by CodeRabbit