Periodic background job to cleanup Windows MDM command queue - #44458
Conversation
![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.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughA cron cleanup job was added to periodically garbage-collect rows in Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/datastore/mysql/microsoft_mdm.go (1)
3445-3451: ⚡ Quick winConsider deterministic ordering in GC batch selection.
Adding an explicit
ORDER BYin the batch subquery makes cleanup progression predictable and prioritizes oldest rows first when runs are interrupted/timeboxed.Proposed change
DELETE q FROM windows_mdm_command_queue q INNER JOIN ( SELECT q2.enrollment_id, q2.command_uuid FROM windows_mdm_command_queue q2 INNER JOIN windows_mdm_command_results r ON r.enrollment_id = q2.enrollment_id AND r.command_uuid = q2.command_uuid WHERE r.created_at < NOW() - INTERVAL 1 HOUR + ORDER BY r.created_at ASC, q2.enrollment_id ASC, q2.command_uuid ASC LIMIT ? ) batch ON batch.enrollment_id = q.enrollment_id AND batch.command_uuid = q.command_uuid🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/datastore/mysql/microsoft_mdm.go` around lines 3445 - 3451, The batch subquery selecting from windows_mdm_command_queue (alias q2) joined to windows_mdm_command_results (alias r) lacks deterministic ordering, so add an explicit ORDER BY to the subquery to make GC progression predictable and prioritize oldest rows first; for example order by r.created_at ASC and include tie-breakers like q2.enrollment_id and q2.command_uuid (i.e., ORDER BY r.created_at ASC, q2.enrollment_id, q2.command_uuid) before the LIMIT ? in the batch subquery.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/datastore/mysql/microsoft_mdm.go`:
- Around line 3445-3451: The batch subquery selecting from
windows_mdm_command_queue (alias q2) joined to windows_mdm_command_results
(alias r) lacks deterministic ordering, so add an explicit ORDER BY to the
subquery to make GC progression predictable and prioritize oldest rows first;
for example order by r.created_at ASC and include tie-breakers like
q2.enrollment_id and q2.command_uuid (i.e., ORDER BY r.created_at ASC,
q2.enrollment_id, q2.command_uuid) before the LIMIT ? in the batch subquery.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: de7c2ed6-06ac-49ab-925d-f3cd9d587457
📒 Files selected for processing (6)
changes/44190-mdm-queue-cleanupcmd/fleet/cron.goserver/datastore/mysql/microsoft_mdm.goserver/datastore/mysql/microsoft_mdm_test.goserver/fleet/datastore.goserver/mock/datastore_mock.go
There was a problem hiding this comment.
Pull request overview
Moves Windows MDM command-queue cleanup off the ACK hot path by keeping queue rows after ACK and introducing a periodic cron-driven garbage-collection step to delete ACKed queue entries later.
Changes:
- Stop deleting
windows_mdm_command_queuerows duringMDMWindowsSaveResponse(ACK processing) and adjust the pending-commands probe to ignore ACKed rows. - Add
CleanupWindowsMDMCommandQueueto the datastore interface, MySQL implementation, mock datastore, and wire it into the hourly cleanups/aggregation cron schedule. - Add/adjust MySQL datastore tests to reflect “queue rows persist after ACK” behavior and to validate the new cleanup logic.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| server/mock/datastore_mock.go | Adds mock datastore hook for CleanupWindowsMDMCommandQueue. |
| server/fleet/datastore.go | Extends Datastore interface with CleanupWindowsMDMCommandQueue. |
| server/datastore/mysql/microsoft_mdm.go | Updates pending-commands probe, removes ACK-time queue DELETE, and adds the cleanup implementation. |
| server/datastore/mysql/microsoft_mdm_test.go | Updates existing ACK/queue assertions and adds a test for the cleanup job behavior. |
| cmd/fleet/cron.go | Registers the new cleanup job in the cleanups/aggregation schedule. |
| changes/44190-mdm-queue-cleanup | Adds user-visible changelog entry for the new periodic cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #44458 +/- ##
==========================================
+ Coverage 66.66% 66.80% +0.14%
==========================================
Files 2651 2641 -10
Lines 213415 212893 -522
Branches 9602 9509 -93
==========================================
- Hits 142267 142221 -46
+ Misses 58211 57711 -500
- Partials 12937 12961 +24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| WHERE r.created_at < NOW() - INTERVAL 1 HOUR | ||
| LIMIT ? | ||
| ) batch ON batch.enrollment_id = q.enrollment_id AND batch.command_uuid = q.command_uuid` | ||
| const maxBatches = 500 |
There was a problem hiding this comment.
500 batches X ~500ms = 4 minutes run time.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
server/datastore/mysql/microsoft_mdm_test.go (1)
5236-5315: ⚡ Quick winExercise the cleanup loop with more than one stale row.
This test verifies the age cutoff, but it still only covers a single deletable row. Given the intended batched GC loop, adding another stale queue row would better catch regressions where the cleaner stops after one batch.
Proposed extension
- // Insert two commands queued for the device. + // Insert three commands queued for the device. cmd1 := &fleet.MDMWindowsCommand{ CommandUUID: uuid.NewString(), RawCommand: []byte(`<Atomic><CmdID>` + uuid.NewString() + `</CmdID></Atomic>`), @@ cmd2 := &fleet.MDMWindowsCommand{ CommandUUID: uuid.NewString(), RawCommand: []byte(`<Atomic><CmdID>` + uuid.NewString() + `</CmdID></Atomic>`), TargetLocURI: "./Device/Test2", } + cmd3 := &fleet.MDMWindowsCommand{ + CommandUUID: uuid.NewString(), + RawCommand: []byte(`<Atomic><CmdID>` + uuid.NewString() + `</CmdID></Atomic>`), + TargetLocURI: "./Device/Test3", + } @@ err = ds.mdmWindowsInsertCommandForHostsDB(ctx, ds.primary, []string{dev.MDMDeviceID}, cmd2) require.NoError(t, err) + err = ds.mdmWindowsInsertCommandForHostsDB(ctx, ds.primary, []string{dev.MDMDeviceID}, cmd3) + require.NoError(t, err) @@ - // Both should be in the queue. + // All three should be in the queue. @@ - require.Equal(t, 2, count) + require.Equal(t, 3, count) @@ - // Insert a result for cmd2 with a recent timestamp (not yet eligible for GC). + // Insert a result for cmd2 with a stale timestamp too. ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { _, err := q.ExecContext(ctx, ` INSERT INTO windows_mdm_command_results (enrollment_id, command_uuid, raw_result, status_code, response_id, created_at) - VALUES (?, ?, '<Status/>', '200', ?, NOW())`, + VALUES (?, ?, '<Status/>', '200', ?, NOW() - INTERVAL 2 HOUR)`, dev.ID, cmd2.CommandUUID, responseID) return err }) + + // Keep cmd3 recent so one row survives the first pass. + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(ctx, ` + INSERT INTO windows_mdm_command_results (enrollment_id, command_uuid, raw_result, status_code, response_id, created_at) + VALUES (?, ?, '<Status/>', '200', ?, NOW())`, + dev.ID, cmd3.CommandUUID, responseID) + return err + }) @@ - // cmd2's queue row should still exist (result is recent). - var cmd2Count int + // cmd2's queue row should be deleted, while cmd3's should remain. + var cmd2Count int ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { return sqlx.GetContext(ctx, q, &cmd2Count, "SELECT COUNT(*) FROM windows_mdm_command_queue WHERE enrollment_id = ? AND command_uuid = ?", dev.ID, cmd2.CommandUUID) }) - assert.Equal(t, 1, cmd2Count, "Queue row for cmd2 should remain (result <1 hour old)") + assert.Equal(t, 0, cmd2Count, "Queue row for cmd2 should be cleaned up (result >1 hour old)") + + var cmd3Count int + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, &cmd3Count, "SELECT COUNT(*) FROM windows_mdm_command_queue WHERE enrollment_id = ? AND command_uuid = ?", + dev.ID, cmd3.CommandUUID) + }) + assert.Equal(t, 1, cmd3Count, "Queue row for cmd3 should remain (result <1 hour old)")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/datastore/mysql/microsoft_mdm_test.go` around lines 5236 - 5315, The test testCleanupWindowsMDMCommandQueue only inserts one deletable command result; to exercise the batched GC loop add a second stale queued command (e.g., create cmd3 with unique CommandUUID and TargetLocURI), insert it via ds.mdmWindowsInsertCommandForHostsDB just like cmd1/cmd2, insert a corresponding windows_mdm_command_results row referencing responseID with created_at older than 1 hour (NOW() - INTERVAL 2 HOUR), then after ds.CleanupWindowsMDMCommandQueue assert that both cmd1 and cmd3 queue rows are gone (count == 0) while cmd2 remains; use the same ExecAdhocSQL/sqlx.GetContext patterns and the existing responseID to create the extra stale result so the test structure and DB constraints are unchanged.server/datastore/mysql/microsoft_mdm.go (1)
3438-3467: Add cleanup telemetry for backlog visibility.Consider emitting metrics for
rows_deletedandbatches_executedper run (and optionally current queue depth) to make alerting on queue growth straightforward.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/datastore/mysql/microsoft_mdm.go` around lines 3438 - 3467, In CleanupWindowsMDMCommandQueue, add telemetry to record rows_deleted and batches_executed (and optionally current queue depth) for each run: instrument inside the loop around ds.writer(ctx).ExecContext so each batch increments a batches_executed counter and adds the returned RowsAffected to rows_deleted; after the loop optionally query COUNT(*) from windows_mdm_command_queue to emit current queue depth; use existing metrics/telemetry utilities in the repo (or add a minimal counter/gauge on the datastore struct) and reference the function name CleanupWindowsMDMCommandQueue, variables stmt, batchSize and maxBatches when adding the metrics calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/datastore/mysql/microsoft_mdm_test.go`:
- Around line 5236-5315: The test testCleanupWindowsMDMCommandQueue only inserts
one deletable command result; to exercise the batched GC loop add a second stale
queued command (e.g., create cmd3 with unique CommandUUID and TargetLocURI),
insert it via ds.mdmWindowsInsertCommandForHostsDB just like cmd1/cmd2, insert a
corresponding windows_mdm_command_results row referencing responseID with
created_at older than 1 hour (NOW() - INTERVAL 2 HOUR), then after
ds.CleanupWindowsMDMCommandQueue assert that both cmd1 and cmd3 queue rows are
gone (count == 0) while cmd2 remains; use the same ExecAdhocSQL/sqlx.GetContext
patterns and the existing responseID to create the extra stale result so the
test structure and DB constraints are unchanged.
In `@server/datastore/mysql/microsoft_mdm.go`:
- Around line 3438-3467: In CleanupWindowsMDMCommandQueue, add telemetry to
record rows_deleted and batches_executed (and optionally current queue depth)
for each run: instrument inside the loop around ds.writer(ctx).ExecContext so
each batch increments a batches_executed counter and adds the returned
RowsAffected to rows_deleted; after the loop optionally query COUNT(*) from
windows_mdm_command_queue to emit current queue depth; use existing
metrics/telemetry utilities in the repo (or add a minimal counter/gauge on the
datastore struct) and reference the function name CleanupWindowsMDMCommandQueue,
variables stmt, batchSize and maxBatches when adding the metrics calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 74a4835a-a47c-4140-8506-21395f9d7e01
📒 Files selected for processing (3)
server/datastore/mysql/microsoft_mdm.goserver/datastore/mysql/microsoft_mdm_test.goserver/fleet/datastore.go
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/datastore/mysql/microsoft_mdm.go (1)
3452-3467: Add saturation observability for cleanup cap.Line 3452 bounds runtime with
maxBatches, which is good for cron safety. Consider emitting a metric/log when the loop exits due to hitting the cap while still deleting full batches (i == maxBatches-1 && n == batchSize) so sustained backlog growth is visible early.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/datastore/mysql/microsoft_mdm.go` around lines 3452 - 3467, The loop in cleanup (using maxBatches, batchSize, stmt and ds.writer(ctx).ExecContext) should emit observability when it exits due to hitting the cap while still returning full batches; add a metric or log when i == maxBatches-1 && n == int64(batchSize) (or immediately before breaking on that condition) to record saturation (e.g., metrics.Counter/Observe or logger.Warn with context including batchSize, maxBatches, stmt or queue name and affected rows) so sustained backlog growth is visible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/datastore/mysql/microsoft_mdm.go`:
- Around line 3452-3467: The loop in cleanup (using maxBatches, batchSize, stmt
and ds.writer(ctx).ExecContext) should emit observability when it exits due to
hitting the cap while still returning full batches; add a metric or log when i
== maxBatches-1 && n == int64(batchSize) (or immediately before breaking on that
condition) to record saturation (e.g., metrics.Counter/Observe or logger.Warn
with context including batchSize, maxBatches, stmt or queue name and affected
rows) so sustained backlog growth is visible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1ec87fec-c6a9-44cd-937b-3b5f907304a3
📒 Files selected for processing (1)
server/datastore/mysql/microsoft_mdm.go
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/datastore/mysql/mdm.go`:
- Around line 96-99: The anti-join currently only matches on command_uuid
(windows_mdm_command_results r WHERE r.enrollment_id = wmcq.enrollment_id AND
r.command_uuid = wmcq.command_uuid) but elsewhere the branch ties results to
hosts via an OR-enrollment join allowing cross-host UUID collisions; revise the
Windows branch to compare the pair (enrollment_id, command_uuid) everywhere:
replace the single-condition join/anti-join with explicit sets keyed by both
enrollment_id and command_uuid (i.e., build the pending queue set alias wmcq and
the results set alias r both filtered on enrollment_id AND command_uuid) or
rewrite as two subqueries (pending rows vs result rows) that join only on both
enrollment_id and command_uuid, mirroring the host-identifier code path so no
results from other enrollments can match.
🪄 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: c10208c0-a9f1-417f-a6a1-749660c28f26
📒 Files selected for processing (2)
server/datastore/mysql/mdm.goserver/datastore/mysql/microsoft_mdm.go
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| LEFT JOIN windows_mdm_command_queue wmcq ON wmcq.command_uuid = wmc.command_uuid | ||
| AND NOT EXISTS ( | ||
| SELECT 1 FROM windows_mdm_command_results r | ||
| WHERE r.enrollment_id = wmcq.enrollment_id AND r.command_uuid = wmcq.command_uuid | ||
| ) |
There was a problem hiding this comment.
In getCombinedMDMCommandsQuery, the Windows branch joins windows_mdm_command_queue by command_uuid only and now adds a correlated NOT EXISTS against windows_mdm_command_results. Since queue rows now persist after ACK, this join will have to examine (and anti-join) potentially large numbers of ACKed queue rows for a command UUID, which can significantly increase the cost of the global ListMDMCommands query as the queue grows. Consider restructuring this Windows query to avoid the unscoped queue join (e.g., UNION a “pending from queue where NOT EXISTS result” query with a “results” query, similar to the host-identifier code path), or otherwise constrain the queue access by enrollment/host earlier so it can use the PK(enrollment_id, command_uuid) efficiently.
There was a problem hiding this comment.
The queue table only holds stale rows for at most ~1 hour. It's not unbounded growth.
getvictor
left a comment
getvictor
left a comment
There was a problem hiding this comment.
Looks good overall. Added a few comments.
| AND NOT EXISTS ( | ||
| SELECT 1 FROM windows_mdm_command_results wmcr | ||
| WHERE wmcr.enrollment_id = wmcq.enrollment_id | ||
| AND wmcr.command_uuid = wmcq.command_uuid | ||
| ) |
There was a problem hiding this comment.
Note. This is extra load on the reader every 1 minute for every host. Probably OK, especially since we expect to use WNS push notifications soon.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
server/datastore/mysql/microsoft_mdm_test.go (1)
5225-5319: ⚡ Quick winAdd a multi-enrollment guard case for cleanup scoping.
testCleanupWindowsMDMCommandQueuecurrently validates one enrollment only. Please add a second enrollment sharing the same queuedcommand_uuidand assert cleanup removes only the stale row for the enrollment with the old result. This hardens against accidental over-delete if cleanup join/filtering regresses.Suggested test extension
func testCleanupWindowsMDMCommandQueue(t *testing.T, ds *Datastore) { ctx := t.Context() dev := createEnrolledDevice(t, ds) + dev2 := createEnrolledDevice(t, ds) // Insert two commands queued for the device. cmd1 := &fleet.MDMWindowsCommand{ CommandUUID: uuid.NewString(), @@ err := ds.mdmWindowsInsertCommandForHostsDB(ctx, ds.primary, []string{dev.MDMDeviceID}, cmd1) require.NoError(t, err) + err = ds.mdmWindowsInsertCommandForHostsDB(ctx, ds.primary, []string{dev2.MDMDeviceID}, cmd1) + require.NoError(t, err) @@ // cmd1's queue row should be deleted (result is >1 hour old). var cmd1Count int ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { return sqlx.GetContext(ctx, q, &cmd1Count, "SELECT COUNT(*) FROM windows_mdm_command_queue WHERE enrollment_id = ? AND command_uuid = ?", dev.ID, cmd1.CommandUUID) }) assert.Equal(t, 0, cmd1Count, "Queue row for cmd1 should be cleaned up (result >1 hour old)") + + // dev2's cmd1 queue row should remain (no stale result for dev2). + var dev2Cmd1Count int + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, &dev2Cmd1Count, "SELECT COUNT(*) FROM windows_mdm_command_queue WHERE enrollment_id = ? AND command_uuid = ?", + dev2.ID, cmd1.CommandUUID) + }) + assert.Equal(t, 1, dev2Cmd1Count, "cleanup should stay scoped to the enrollment that has a stale result") }As per coding guidelines, queries intended for specific entities should be verified for precise scoping to avoid unintended results.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/datastore/mysql/microsoft_mdm_test.go` around lines 5225 - 5319, The test testCleanupWindowsMDMCommandQueue needs a second enrollment to ensure CleanupWindowsMDMCommandQueue only removes the stale queue row for the specific enrollment; add a new enrolled device (e.g., createEnrolledDevice call to produce dev2), insert a queued command for dev2 using ds.mdmWindowsInsertCommandForHostsDB with the same CommandUUID as cmd1, and do NOT insert an old command result for dev2 (or insert a recent result) so that after calling ds.CleanupWindowsMDMCommandQueue you assert the stale queue row for dev (cmd1) is deleted while the queue row for dev2 with the same command_uuid remains; scope your additional assertions by querying windows_mdm_command_queue filtering on enrollment_id and command_uuid (same pattern used for cmd1Count/cmd2Count/cmd3Count).server/datastore/mysql/microsoft_mdm.go (1)
3446-3464: Consider surfacing backlog or cap-hit metrics here.The warning only fires after a tick has already spent its full 500k-row budget. A gauge/counter for queue depth or consecutive capped runs would make it much easier to alert before backlog growth becomes user-visible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/datastore/mysql/microsoft_mdm.go` around lines 3446 - 3464, Add metrics to surface backlog/cap hits around the cleanup loop: emit a queue-depth gauge (e.g. set windows_mdm_queue_depth) and a capped-run counter (e.g. windows_mdm_capped_runs) using the datastore's metrics facility inside the loop or immediately after it; use known symbols maxBatches, totalDeleted, exhausted, stmt, batchSize and the writer.ExecContext call to determine and report progress (set gauge to remaining rows if you can query count or approximate it from totalDeleted and batchSize) and increment windows_mdm_capped_runs when exhausted is true before calling ds.logger.WarnContext so alerts can trigger earlier than the log warning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/datastore/mysql/microsoft_mdm_test.go`:
- Around line 5225-5319: The test testCleanupWindowsMDMCommandQueue needs a
second enrollment to ensure CleanupWindowsMDMCommandQueue only removes the stale
queue row for the specific enrollment; add a new enrolled device (e.g.,
createEnrolledDevice call to produce dev2), insert a queued command for dev2
using ds.mdmWindowsInsertCommandForHostsDB with the same CommandUUID as cmd1,
and do NOT insert an old command result for dev2 (or insert a recent result) so
that after calling ds.CleanupWindowsMDMCommandQueue you assert the stale queue
row for dev (cmd1) is deleted while the queue row for dev2 with the same
command_uuid remains; scope your additional assertions by querying
windows_mdm_command_queue filtering on enrollment_id and command_uuid (same
pattern used for cmd1Count/cmd2Count/cmd3Count).
In `@server/datastore/mysql/microsoft_mdm.go`:
- Around line 3446-3464: Add metrics to surface backlog/cap hits around the
cleanup loop: emit a queue-depth gauge (e.g. set windows_mdm_queue_depth) and a
capped-run counter (e.g. windows_mdm_capped_runs) using the datastore's metrics
facility inside the loop or immediately after it; use known symbols maxBatches,
totalDeleted, exhausted, stmt, batchSize and the writer.ExecContext call to
determine and report progress (set gauge to remaining rows if you can query
count or approximate it from totalDeleted and batchSize) and increment
windows_mdm_capped_runs when exhausted is true before calling
ds.logger.WarnContext so alerts can trigger earlier than the log warning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bf5a3a20-36ab-4a7a-8cb3-6b09beb4eac0
📒 Files selected for processing (3)
server/datastore/mysql/mdm.goserver/datastore/mysql/microsoft_mdm.goserver/datastore/mysql/microsoft_mdm_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- server/datastore/mysql/mdm.go
Related issue: Resolves #44190
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.Testing
Summary by CodeRabbit
New Features
Bug Fixes
Tests