Skip to content

Fixed slow migration. - #50992

Merged
getvictor merged 2 commits into
mainfrom
50933-migration
Aug 11, 2026
Merged

Fixed slow migration.#50992
getvictor merged 2 commits into
mainfrom
50933-migration

Conversation

@getvictor

@getvictor getvictor commented Aug 11, 2026

Copy link
Copy Markdown
Member

Related issue: Resolves #50933

Checklist for submitter

If some of the following don't apply, delete the relevant line.

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

Testing

  • QA'd all new/changed functionality manually

Summary by CodeRabbit

  • Bug Fixes
    • Improved the performance of database migrations for deployments with many Windows host certificates.
    • Reduced delays when identifying certificates for soft deletion during migration.

@getvictor
getvictor requested a lite review from Copilot August 11, 2026 18:42
@getvictor

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

Copilot AI 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.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Pull request overview

This PR addresses the long-running Windows host certificate migration reported in #50933 by adjusting the migration’s batch-selection query to encourage a more efficient index choice during iteration.

Changes:

  • Adds an explicit MySQL index hint (FORCE INDEX (PRIMARY)) to the batched SELECT hc.id query used during the incremental soft-delete loop.
  • Includes a user-visible changes entry in changes/ (content excluded from review output per policy).

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File Description
server/datastore/mysql/migrations/tables/20260723181402_ReparseWindowsHostCertificates.go Forces primary-key index usage for the batch ID scan to improve migration performance on large datasets.
changes/50933-slow-windows-host-certificate-migration User-visible change log entry (content excluded by policy).
Files excluded by content exclusion policy (1)
  • changes/50933-slow-windows-host-certificate-migration

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov

codecov Bot commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.56%. Comparing base (79305ea) to head (6bd8689).
⚠️ Report is 26 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #50992      +/-   ##
==========================================
+ Coverage   68.53%   68.56%   +0.02%     
==========================================
  Files        3978     3985       +7     
  Lines      256274   256669     +395     
  Branches    13627    13627              
==========================================
+ Hits       175632   175977     +345     
- Misses      65006    65046      +40     
- Partials    15636    15646      +10     
Flag Coverage Δ
backend 69.66% <100.00%> (+0.03%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

@getvictor
getvictor marked this pull request as ready for review August 11, 2026 18:52
@getvictor
getvictor requested a review from a team as a code owner August 11, 2026 18:52
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The Windows host certificate migration now forces MySQL to use the PRIMARY index when selecting certificate IDs for soft deletion. Existing filtering, batching, updates, progress reporting, and error handling remain unchanged. A changelog entry documents the migration performance fix for deployments with many host certificates.

Possibly related PRs

  • fleetdm/fleet#49849: Modifies the same migration but changes its timestamp rather than its index usage.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The migration now uses the primary-key index for certificate batching, directly addressing the long-running migration reported in [#50933].
Out of Scope Changes check ✅ Passed The changes are limited to the migration performance fix and its user-visible changelog entry.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title clearly identifies the primary change: fixing a slow migration.
Description check ✅ Passed The description identifies the related issue, confirms the changes file, and records manual QA for the migration fix.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 50933-migration

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sharon-fdm sharon-fdm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. 🚢

@getvictor
getvictor merged commit 1f923ad into main Aug 11, 2026
45 of 46 checks passed
@getvictor
getvictor deleted the 50933-migration branch August 11, 2026 19:35
getvictor added a commit that referenced this pull request Aug 11, 2026
<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #50933

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

## Testing

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

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Bug Fixes**
- Improved the performance of database migrations for deployments with
many Windows host certificates.
- Reduced delays when identifying certificates for soft deletion during
migration.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

(cherry picked from commit 1f923ad)
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.

v4.89.2 -> v4.90.0 -> Long running migration(s)

3 participants