Lucas Manuel Rodriguez
bec3b0dc2a
<!-- 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 -->
2 lines
260 B
Plaintext
2 lines
260 B
Plaintext
* Reduced MySQL reader load when listing hosts with `device_mapping=true` and a search query by evaluating device mapping as a per-row correlated subquery instead of a fully-materialized derived-table join, and by skipping it entirely in the host count query.
|