Throttle requests to AMAPI during profile reconcilation - #47223
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces batched Android MDM profile reconciliation to reduce load on the Google Android Management API (AMAPI) by limiting how many hosts are processed per cron tick, using a persisted cursor to advance across ticks.
Changes:
- Add a configurable
mdm.android_profiles_batch_size(envFLEET_MDM_ANDROID_PROFILES_BATCH_SIZE, default 1000) and thread it into the Android profile manager cron. - Add a Redis-backed reconcile cursor (
Get/SetMDMAndroidReconcileCursor) and use it inReconcileProfilesWithClientto advance pagination across runs. - Extend
ListMDMAndroidProfilesToSendto accept(cursor, batchSize)so the datastore can return only the next window of hosts.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| server/service/integration_mdm_test.go | Updates call site for new ReconcileProfilesWithClient(..., batchSize) signature. |
| server/mock/datastore_mock.go | Updates mock datastore for new cursor methods and updated list signature. |
| server/mdm/android/service/profiles.go | Implements cursor read/advance logic and passes batching params into reconciliation. |
| server/mdm/android/service/profiles_test.go | Updates tests to match new reconciler signature. |
| server/fleet/datastore.go | Extends datastore interface with cursor methods and list paging parameters. |
| server/datastore/mysqlredis/android_recon_cursor.go | Adds Redis-backed cursor persistence for Android reconcile. |
| server/datastore/mysql/android.go | Implements cursor/batchSize support in ListMDMAndroidProfilesToSend. |
| server/datastore/mysql/android_test.go | Updates tests for new list method signature. |
| server/config/config.go | Adds config field + config manager registration for Android profile batch size. |
| cmd/fleet/serve.go | Passes configured batch size into Android profile manager schedule. |
| cmd/fleet/cron.go | Threads batchSize through the Android profile manager cron job. |
| changes/41910-throttle-AMAPI | Documents the new batch-size configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
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:
WalkthroughThis PR adds a configurable AndroidProfilesBatchSize (default 1000), updates datastore and mock APIs to support cursor-based pagination (ListMDMAndroidProfilesToSend(ctx, cursor, batchSize)), implements Redis-backed cursor persistence, changes the MySQL query to respect cursor and limit, extends reconciliation to run per-cursor batches and advance the cursor on success, and wires the batch size through the cron job. Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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.
Actionable comments posted: 2
🤖 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/config/config.go`:
- Line 2093: The AndroidProfilesBatchSize value is assigned from
man.getConfigInt without validation; add a non-negative check by implementing a
method on MDMConfig, e.g., ValidateAndroidProfilesBatchSize(initFatal func(err
error, msg string)), that calls initFatal if m.AndroidProfilesBatchSize < 0 with
a clear error message stating "mdm.android_profiles_batch_size must be
non-negative (0 means no limit)"; then invoke
MDMConfig.ValidateAndroidProfilesBatchSize during the config
initialization/validation phase alongside other MDM validations so the invalid
value is caught early.
- Around line 1759-1760: The config key mdm.android_profiles_batch_size is
hidden with man.hideConfig after being registered via man.addConfigInt so
operators cannot discover it; add documentation for this key in the
operator-facing configuration reference or deployment docs (or document its
equivalent env var) describing its purpose, default (1000), permitted values
(integer, 0 = no limit), and when to adjust it. Locate the registration calls
(man.addConfigInt("mdm.android_profiles_batch_size", 1000, ...) and
man.hideConfig("mdm.android_profiles_batch_size")) to ensure the doc text
matches the code behavior and update the docs site or deployment README to
include this entry. Ensure the documentation mentions the changelog reference
(changes/41910-throttle-AMAPI) for operator context.
🪄 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: 60314b28-4b4c-40c1-9116-124cea091b58
📒 Files selected for processing (12)
changes/41910-throttle-AMAPIcmd/fleet/cron.gocmd/fleet/serve.goserver/config/config.goserver/datastore/mysql/android.goserver/datastore/mysql/android_test.goserver/datastore/mysqlredis/android_recon_cursor.goserver/fleet/datastore.goserver/mdm/android/service/profiles.goserver/mdm/android/service/profiles_test.goserver/mock/datastore_mock.goserver/service/integration_mdm_test.go
| man.addConfigInt("mdm.android_profiles_batch_size", 1000, "Maximum number of hosts to process per Android profile reconciliation tick (1000 default; 0 no limit)") | ||
| man.hideConfig("mdm.android_profiles_batch_size") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if android_profiles_batch_size is documented in user-facing docs
# Search for documentation of this config option
rg -n -C3 -i 'android.*profile.*batch' --type=mdRepository: fleetdm/fleet
Length of output: 39
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Search for exact config key
rg -n --hidden --no-ignore -S "mdm\.android_profiles_batch_size" . || true
# 2) Search for alternative spellings / token without namespace
rg -n --hidden --no-ignore -S "android_profiles_batch_size" . || true
# 3) Search for human-readable description variants
rg -n --hidden --no-ignore -i "android.*profiles.*batch.*size|batch.*size.*android.*profiles|android profile.*batch" . || trueRepository: fleetdm/fleet
Length of output: 3316
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for env var name and config key mentions across docs/help content
rg -n --hidden --no-ignore -S "FLEET_MDM_ANDROID_PROFILES_BATCH_SIZE|mdm\.android_profiles_batch_size|android_profiles_batch_size" . \
-g'*.md' -g'*.adoc' -g'*.rst' -g'*.txt' -g'*.go' -g'*.yaml' -g'*.yml' -g'*.json' -g'*.html' -g'*.mdx' || trueRepository: fleetdm/fleet
Length of output: 559
🌐 Web query:
FLEET_MDM_ANDROID_PROFILES_BATCH_SIZE fleetdm documentation
💡 Result:
I couldn’t find any Fleet documentation (or code/doc reference surfaced by search) that defines or even mentions the constant/setting name “FLEET_MDM_ANDROID_PROFILES_BATCH_SIZE”. The only Fleet “batch” profile-related references I found are for the HTTP endpoints like POST /api/v1/fleet/mdm/profiles/batch (and related config-profile batch endpoints), but they don’t expose a documented environment variable by that name in the retrieved sources.[1][2][3] If you meant the batch size used internally for Android MDM profiles, it is likely implemented as a backend constant or derived from configuration in the Fleet codebase rather than exposed as “FLEET_MDM_ANDROID_PROFILES_BATCH_SIZE”. From the sources retrieved here, the most concrete and documentable item is that there is a batch-set endpoint for MDM profiles (/api/v1/fleet/mdm/profiles/batch), and it supports a dry_run mode for validation without applying changes.[3][2] To proceed: tell me whether you’re looking for (a) an environment variable name, (b) the max number of profiles accepted by the batch endpoint, or (c) the internal DB/insert batching used when applying Android profiles. With that clarification I can target the exact doc/code location.
Citations:
- 1: Batch-modify configuration profiles: Add Android and update error messages #32786
- 2: Profiles batch activity #21604
- 3: CLI: Windows profiles #14361
🌐 Web query:
mdm.android_profiles_batch_size fleetdm
💡 Result:
I couldn’t find any FleetDM documentation or code that defines a setting named mdm.android_profiles_batch_size (or any value for it) in the sources I searched. What I did find is that Fleet has a “batch-modify configuration profiles” workflow for MDM profiles, exercised via the POST /configuration_profiles/batch API endpoint for Android JSON profiles, with internal load-testing notes around how many profiles are practical per request [1]. However, that is not the same as a configurable mdm.android_profiles_batch_size parameter. If you can share where you saw mdm.android_profiles_batch_size (e.g., a config file, Helm chart, or environment variable list), I can re-search specifically for that exact key/value in Fleet’s repo/config docs.
Citations:
🌐 Web query:
android_profiles_batch_size fleetdm
💡 Result:
FleetDM does not appear to document or expose a configuration knob named android_profiles_batch_size in its public docs/code references; the batch sizing behavior discussed in Fleet for Android profiles is implemented as throttling/limits on how many hosts and/or profiles are processed per reconciliation/installation run, rather than a clearly named android_profiles_batch_size setting. Specifically, Fleet maintainers opened work items to “limit software and profiles host count for each run, to reduce Android API usage” [1] and to limit how many devices an instance can process in each Android profile reconciliation and software installation run “to avoid overloading the android API” [2]. Another issue proposes exploring batching/throttling (e.g., “deliver profiles to 10k hosts every 30 seconds”) to manage load at scale [3]. If you’re trying to find the exact setting value/name in your Fleet deployment, the best-supported approach from available sources is to look for the implementation of the “limit … host count for each run” / “limit … devices … each profile reconciliation run” logic in the Fleet server (GitHub) rather than expecting a config parameter called android_profiles_batch_size [2][1]. Answering your query literally (“android_profiles_batch_size fleetdm”): no authoritative evidence surfaced that such a parameter exists or is documented publicly, and the referenced scaling controls are described in terms of limiting host/device counts per run [2][1][3].
Citations:
- 1: Limit software and profiles host count for each run, to reduce Android API usage #41907
- 2: Limit Android Profile reconciler and software installation #41910
- 3: Properly scale MDM profile delivery for 60k+ hosts #24428
Document hidden mdm.android_profiles_batch_size in operator-facing docs
mdm.android_profiles_batch_size is hidden via man.hideConfig and appears to be undocumented beyond server/config/config.go (and a changelog entry in changes/41910-throttle-AMAPI). Add it to the configuration reference/deployment documentation (or document the corresponding env var) so operators can discover and use it.
🤖 Prompt for 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.
In `@server/config/config.go` around lines 1759 - 1760, The config key
mdm.android_profiles_batch_size is hidden with man.hideConfig after being
registered via man.addConfigInt so operators cannot discover it; add
documentation for this key in the operator-facing configuration reference or
deployment docs (or document its equivalent env var) describing its purpose,
default (1000), permitted values (integer, 0 = no limit), and when to adjust it.
Locate the registration calls
(man.addConfigInt("mdm.android_profiles_batch_size", 1000, ...) and
man.hideConfig("mdm.android_profiles_batch_size")) to ensure the doc text
matches the code behavior and update the docs site or deployment README to
include this entry. Ensure the documentation mentions the changelog reference
(changes/41910-throttle-AMAPI) for operator context.
There was a problem hiding this comment.
I don't want customers casually tuning this setting since all traffic goes through the shared proxy (fleetdm.com). But operators who need the escape valve can still set FLEET_MDM_ANDROID_PROFILES_BATCH_SIZE via env var. Hidden configs are still functional, they just don't show up in fleetctl config output or public docs.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #47223 +/- ##
==========================================
+ Coverage 67.15% 67.24% +0.08%
==========================================
Files 2970 2977 +7
Lines 226456 227487 +1031
Branches 11868 11868
==========================================
+ Hits 152075 152971 +896
- Misses 60641 60683 +42
- Partials 13740 13833 +93
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:
|
| @@ -0,0 +1 @@ | |||
| - Added configurable batch size `FLEET_MDM_ANDROID_PROFILES_BATCH_SIZE` (default: 1000 hosts) for Android MDM profile reconciliation to prevent overwhelming the Google Android Management API. | |||
There was a problem hiding this comment.
Will open another PR to deal with software installation throttling. Grouping this into one PR is too hefty. 👍
getvictor
left a comment
getvictor
left a comment
There was a problem hiding this comment.
Thanks for this change. I made a few comments. Feel free to merge and do a follow-up PR to fix any issues.
| hostCount, err := reconciler.ReconcileProfiles(ctx, cursor, batchSize) | ||
|
|
||
| var nextCursor string | ||
| if batchSize > 0 && hostCount >= batchSize { |
There was a problem hiding this comment.
Should this be hostCount > batchSize?
There was a problem hiding this comment.
This matches the Windows pattern at microsoft_mdm.go:3524
if len(hostUUIDs) >= reconcileWindowsProfilesBatchSize {
nextCursor = hostUUIDs[len(hostUUIDs)-1]
}
With > getting exactly batchSize hosts would reset the cursor, causing the next tick to restart from the beginning and re-process those same hosts before continuing.
Related issue: Resolves #41910
Checklist for submitter
If some of the following don't apply, delete the relevant line.
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