Skip to content

Batch select query in CleanupExcessQueryResultRows - #40491

Merged
nulmete merged 3 commits into
mainfrom
nulmete/batch-select-query-cleanup-excess-query-results
Feb 25, 2026
Merged

Batch select query in CleanupExcessQueryResultRows#40491
nulmete merged 3 commits into
mainfrom
nulmete/batch-select-query-cleanup-excess-query-results

Conversation

@nulmete

@nulmete nulmete commented Feb 25, 2026

Copy link
Copy Markdown
Member

Related issue: Resolves #40476

Checklist for submitter

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
    See Changes files for more information.

Testing

  • Added/updated automated tests

  • QA'd all new/changed functionality manually

Before:

  • inserted 70k queries to my local DB, saw the cron failing:
INSERT INTO queries (name, description, query, author_id, logging_type, discard_data, saved)
  SELECT
    CONCAT('bulk_query_', seq),
    '',
    'SELECT 1',
    (SELECT id FROM users LIMIT 1),
    'snapshot',
    false,
    1
  FROM (
    SELECT a.N + b.N*10 + c.N*100 + d.N*1000 + e.N*10000 as seq
    FROM
      (SELECT 0 AS N UNION SELECT 1 UNION SELECT 2 UNION SELECT 3 UNION SELECT 4 UNION SELECT 5 UNION SELECT 6
  UNION SELECT 7 UNION SELECT 8 UNION SELECT 9) a,
      (SELECT 0 AS N UNION SELECT 1 UNION SELECT 2 UNION SELECT 3 UNION SELECT 4 UNION SELECT 5 UNION SELECT 6
  UNION SELECT 7 UNION SELECT 8 UNION SELECT 9) b,
      (SELECT 0 AS N UNION SELECT 1 UNION SELECT 2 UNION SELECT 3 UNION SELECT 4 UNION SELECT 5 UNION SELECT 6
  UNION SELECT 7 UNION SELECT 8 UNION SELECT 9) c,
      (SELECT 0 AS N UNION SELECT 1 UNION SELECT 2 UNION SELECT 3 UNION SELECT 4 UNION SELECT 5 UNION SELECT 6
  UNION SELECT 7 UNION SELECT 8 UNION SELECT 9) d,
      (SELECT 0 AS N UNION SELECT 1 UNION SELECT 2 UNION SELECT 3 UNION SELECT 4 UNION SELECT 5 UNION SELECT 6
  UNION SELECT 7 UNION SELECT 8 UNION SELECT 9) e
  ) numbers
  WHERE seq < 70000;
Screenshot 2026-02-25 at 12 54 31 PM
  • ran new test without code fix, it failed with the same error in the issue:
Screenshot 2026-02-25 at 12 45 41 PM

After: ran test again, it passed

Screenshot 2026-02-25 at 12 45 04 PM

@nulmete nulmete changed the title Batch query in CleanupExcessQueryResultRows Batch select query in CleanupExcessQueryResultRows Feb 25, 2026
@codecov

codecov Bot commented Feb 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 55.55556% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.27%. Comparing base (a3166c4) to head (9b0db93).
⚠️ Report is 30 commits behind head on main.

Files with missing lines Patch % Lines
server/datastore/mysql/query_results.go 55.55% 4 Missing and 4 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #40491   +/-   ##
=======================================
  Coverage   66.26%   66.27%           
=======================================
  Files        2461     2461           
  Lines      197427   197434    +7     
  Branches     8655     8655           
=======================================
+ Hits       130832   130847   +15     
+ Misses      54749    54745    -4     
+ Partials    11846    11842    -4     
Flag Coverage Δ
backend 68.14% <55.55%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sgress454 sgress454 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good but I think the bigger fix is to update the SQL for the initial QueryIDs statement here to include saved=1 in the WHERE clause. Live reports don't save rows in the query_results table anyway so there's nothing to clean up for them, and we're wasting cycles fetching them.

Your added batching is still a good idea, just in case a customer really does have >65k saved reports, but it should rarely be used if we fix the filtering.

@nulmete
nulmete marked this pull request as ready for review February 25, 2026 17:24
@nulmete
nulmete requested a review from a team as a code owner February 25, 2026 17:24
@nulmete
nulmete requested a review from sgress454 February 25, 2026 17:24
@nulmete
nulmete merged commit 09d86aa into main Feb 25, 2026
46 checks passed
@nulmete
nulmete deleted the nulmete/batch-select-query-cleanup-excess-query-results branch February 25, 2026 21:01
georgekarrv pushed a commit that referenced this pull request Feb 27, 2026
<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #40476

# Checklist for submitter

- [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.

## Testing

- [x] Added/updated automated tests

- [x] QA'd all new/changed functionality manually

Before:
- inserted 70k queries to my local DB, saw the cron failing:

<img width="864" height="120" alt="Screenshot 2026-02-25 at 12 54 31 PM"
src="https://github.com/user-attachments/assets/d1e19aa8-56aa-46a2-a437-7ae5da1e5b1e"
/>

- ran new test without code fix, it failed with the same error in the
issue:

<img width="920" height="324" alt="Screenshot 2026-02-25 at 12 45 41 PM"
src="https://github.com/user-attachments/assets/c7342d81-f223-449e-a861-c7bae58bbe9e"
/>

After: ran test again, it passed

<img width="1556" height="174" alt="Screenshot 2026-02-25 at 12 45
04 PM"
src="https://github.com/user-attachments/assets/9eed3e6e-3ce6-4d69-aa70-9ebcfcf07623"
/>
georgekarrv pushed a commit that referenced this pull request Mar 3, 2026
<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #40476

# Checklist for submitter

- [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.

## Testing

- [x] Added/updated automated tests

- [x] QA'd all new/changed functionality manually

Before:
- inserted 70k queries to my local DB, saw the cron failing:

<img width="864" height="120" alt="Screenshot 2026-02-25 at 12 54 31 PM"
src="https://github.com/user-attachments/assets/d1e19aa8-56aa-46a2-a437-7ae5da1e5b1e"
/>

- ran new test without code fix, it failed with the same error in the
issue:

<img width="920" height="324" alt="Screenshot 2026-02-25 at 12 45 41 PM"
src="https://github.com/user-attachments/assets/c7342d81-f223-449e-a861-c7bae58bbe9e"
/>

After: ran test again, it passed

<img width="1556" height="174" alt="Screenshot 2026-02-25 at 12 45
04 PM"
src="https://github.com/user-attachments/assets/9eed3e6e-3ce6-4d69-aa70-9ebcfcf07623"
/>
This was referenced Mar 3, 2026
@georgekarrv georgekarrv mentioned this pull request Mar 6, 2026
georgekarrv added a commit that referenced this pull request Mar 10, 2026
- **Batch select query in CleanupExcessQueryResultRows (#40491)**
- **Cherry-pick: Remove "do not enqueue setup experience items >24 hours
after enrollment" logic for macOS hosts (#40739) (#40748)**

---------

Co-authored-by: Nico <32375741+nulmete@users.noreply.github.com>
Co-authored-by: Sarah Gillespie <73313222+gillespi314@users.noreply.github.com>
Co-authored-by: Jordan Montgomery <elijah.jordan.montgomery@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fleet v4.81.0 Query Results Cleanup Error

2 participants