<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #47722 The issue was from a customer running `GET /api/v1/fleet/hosts?device_mapping=true&page=1&per_page=100&query=<ADDRESS>%40example.com` on a script in a for loop. This change reduces the impact of the API on such workflows. Results from my local load test: EXPLAIN ANALYZE: ``` ┌───────────────────────────────────┬────────────────┬─────────────┬─────────────────────────────────────────────┐ │ │ optimizer cost │ actual time │ device_mapping aggregation │ ├───────────────────────────────────┼────────────────┼─────────────┼─────────────────────────────────────────────┤ │ Old (derived-table GROUP BY join) │ ~23,179 │ ~73 ms │ materialized dm derived table, cost ~7,125 │ ├───────────────────────────────────┼────────────────┼─────────────┼─────────────────────────────────────────────┤ │ New (correlated subquery) │ ~1,260 │ ~25 ms │ Aggregate … loops=1 (only the returned row) │ └───────────────────────────────────┴────────────────┴─────────────┴─────────────────────────────────────────────┘ ``` Tests with 10k hosts: ``` ┌───────────────────────────────────┬────────────┬───────────────┬───────┐ │ dataset │ OLD (main) │ NEW (this PR) │ ratio │ ├───────────────────────────────────┼────────────┼───────────────┼───────┤ │ 10k hosts × 3 emails (30k rows) │ 4.6s │ 1.1s │ ~4× │ ├───────────────────────────────────┼────────────┼───────────────┼───────┤ │ 10k hosts × 30 emails (300k rows) │ 35.9s │ 1.2s │ ~30× │ └───────────────────────────────────┴────────────┴───────────────┴───────┘ ``` # 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), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. ## Testing - [x] Added/updated automated tests - [x] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually ## What & why `GET /api/v1/fleet/hosts?device_mapping=true&page=1&per_page=100&query=<email>` caused high MySQL **reader** load on instances with ~10k hosts. Each page load ran an expensive aggregation over the entire `host_emails` table even though only ~100 rows are returned. **Root cause:** with `device_mapping=true`, `applyHostFilters` added a `LEFT JOIN` on a derived table with `GROUP BY host_id` over `host_emails`. Because of the `GROUP BY`, MySQL must fully materialize that derived table (aggregating every row for all hosts) before the outer `WHERE`/`LIMIT 100` can be applied, so the full cost is paid on every page request regardless of result size. `CountHosts` reused the same options, materializing the aggregation a **second** time per page load. **Fixes (both in `server/datastore/mysql/hosts.go`):** 1. Replaced the derived-table join with a correlated subquery in the `SELECT` list (only when `opt.DeviceMapping`), so it is evaluated only for the rows actually returned, each as an indexed lookup on `idx_host_emails_host_id_email`. This matches the existing `host_additional` pattern in the same query. 2. Set `opt.DeviceMapping = false` in `CountHosts` — the column is never selected for counting — mirroring the existing `opt.DisableIssues` handling. ## Notes - The composite index `idx_host_emails_host_id_email (host_id, email)` already exists, so the correlated subquery resolves via an indexed lookup per returned row. - `TestHosts` (full suite) passes, including `HostDeviceMapping`, `CustomHostDeviceMapping`, and `IDPHostDeviceMapping` (the last two verify the `custom_*` → `custom` and `idp` → `mdm_idp_accounts` source translation still works through the new subquery). - Recommend validating with `EXPLAIN ANALYZE` on a ~10k-host dataset before/after, per the issue. I did not have access to such a dataset. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Performance** * Improved host list responsiveness when using search filters alongside device mapping. * Reduced database load during host listing by retrieving device mapping more efficiently per host. * Improved host counting speed by avoiding device-mapping evaluation for count queries. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -1161,9 +1161,17 @@ func (ds *Datastore) ListHosts(ctx context.Context, filter fleet.TeamFilter, opt
|
||||
sql += hostMDMSelect
|
||||
|
||||
if opt.DeviceMapping {
|
||||
sql += `,
|
||||
COALESCE(dm.device_mapping, 'null') as device_mapping
|
||||
`
|
||||
// Use a correlated subquery in the SELECT list (rather than a derived-table
|
||||
// LEFT JOIN with GROUP BY) so the aggregation over host_emails is evaluated only
|
||||
// for the rows actually returned by the outer query, each as an indexed lookup on
|
||||
// idx_host_emails_host_id_email.
|
||||
sql += fmt.Sprintf(`,
|
||||
COALESCE((
|
||||
SELECT CONCAT('[', GROUP_CONCAT(JSON_OBJECT('email', he.email, 'source', %s)), ']')
|
||||
FROM host_emails he
|
||||
WHERE he.host_id = h.id
|
||||
), 'null') as device_mapping
|
||||
`, deviceMappingTranslateSourceColumn("he"))
|
||||
}
|
||||
|
||||
if !opt.DisableIssues {
|
||||
@@ -1305,18 +1313,6 @@ func (ds *Datastore) applyHostFilters(
|
||||
// prior to returning, params will be appended in the following order: selectParams, joinParams, whereParams
|
||||
var whereParams, joinParams []interface{}
|
||||
|
||||
deviceMappingJoin := fmt.Sprintf(`LEFT JOIN (
|
||||
SELECT
|
||||
host_id,
|
||||
CONCAT('[', GROUP_CONCAT(JSON_OBJECT('email', email, 'source', %s)), ']') AS device_mapping
|
||||
FROM
|
||||
host_emails
|
||||
GROUP BY
|
||||
host_id) dm ON dm.host_id = h.id`, deviceMappingTranslateSourceColumn(""))
|
||||
if !opt.DeviceMapping {
|
||||
deviceMappingJoin = ""
|
||||
}
|
||||
|
||||
policyMembershipJoin := "JOIN policy_membership pm ON (h.id = pm.host_id)"
|
||||
if opt.PolicyIDFilter == nil {
|
||||
policyMembershipJoin = ""
|
||||
@@ -1484,7 +1480,6 @@ func (ds *Datastore) applyHostFilters(
|
||||
%s
|
||||
%s
|
||||
%s
|
||||
%s
|
||||
%s
|
||||
%s
|
||||
%s
|
||||
@@ -1493,7 +1488,6 @@ func (ds *Datastore) applyHostFilters(
|
||||
|
||||
// JOINs
|
||||
hostMDMJoin,
|
||||
deviceMappingJoin,
|
||||
policyMembershipJoin,
|
||||
softwareStatusJoin,
|
||||
failingPoliciesJoin,
|
||||
@@ -2093,6 +2087,8 @@ func (ds *Datastore) CountHosts(ctx context.Context, filter fleet.TeamFilter, op
|
||||
opt.PerPage = 0
|
||||
// We don't need the issue counts of each host for counting hosts.
|
||||
opt.DisableIssues = true
|
||||
// device_mapping is never selected when counting, so skip its (expensive) subquery.
|
||||
opt.DeviceMapping = false
|
||||
|
||||
var params []interface{}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user