Skip to content

Fixed dead rows accumulating in software host counts tables#40288

Merged
getvictor merged 8 commits intomainfrom
victor/35805-zero-host-counts
Feb 24, 2026
Merged

Fixed dead rows accumulating in software host counts tables#40288
getvictor merged 8 commits intomainfrom
victor/35805-zero-host-counts

Conversation

@getvictor
Copy link
Copy Markdown
Member

@getvictor getvictor commented Feb 23, 2026

Related issue: Resolves #35805

Fixed to make sure software host counts tables never have host counts of 0.
Planning to loadtest this fix along with the follow up fix for #35799

Checklist for submitter

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

Testing

  • Added/updated automated tests
  • QA'd all new/changed functionality manually

Summary by CodeRabbit

  • Bug Fixes
    • Fixed accumulation of dead rows in software host count tracking, improving data accuracy and system performance.
    • Enhanced validation to ensure consistent and reliable software availability records.

@getvictor
Copy link
Copy Markdown
Member Author

@coderabbitai full review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses accumulation of “dead” hosts_count = 0 rows in the host-count aggregation tables by switching the sync logic to an atomic swap-table approach and enforcing positive counts at the schema level.

Changes:

  • Reworked SyncHostsSoftware and SyncHostsSoftwareTitles to populate swap tables and atomically rename them into place (instead of resetting counts to 0 and updating in-place).
  • Updated listing/filtering SQL to no longer rely on hosts_count > 0 predicates, consistent with eliminating zero-count rows.
  • Added a migration + schema constraints to delete existing zero-count rows and prevent future inserts of non-positive counts; adjusted unit test expectations accordingly.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
server/datastore/mysql/software.go Uses swap-table + rename for software_host_counts; removes reliance on hosts_count > 0 filtering in several queries.
server/datastore/mysql/software_titles.go Uses swap-table + rename for software_titles_host_counts; adjusts title queries to check row existence instead of hosts_count > 0.
server/datastore/mysql/migrations/tables/20260223000000_CleanupSoftwareHostCountsZeroRows.go Cleans up accumulated zero rows and adds CHECK constraints to enforce hosts_count > 0.
server/datastore/mysql/schema.sql Reflects new constraints and migration status in the canonical schema.
server/datastore/mysql/software_test.go Updates expected row counts in software_host_counts due to removal of zero-count rows.
changes/35805-zero-host-counts Adds changelog entry for the bugfix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 23, 2026

Codecov Report

❌ Patch coverage is 68.29268% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.54%. Comparing base (81c75b4) to head (1f7dc61).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
server/datastore/mysql/software.go 58.82% 8 Missing and 6 partials ⚠️
server/datastore/mysql/software_titles.go 60.00% 5 Missing and 5 partials ⚠️
...0260223000000_CleanupSoftwareHostCountsZeroRows.go 91.30% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #40288      +/-   ##
==========================================
- Coverage   66.26%   65.54%   -0.73%     
==========================================
  Files        2460     2274     -186     
  Lines      197220   180111   -17109     
  Branches     8716     8593     -123     
==========================================
- Hits       130682   118046   -12636     
+ Misses      54713    50865    -3848     
+ Partials    11825    11200     -625     
Flag Coverage Δ
backend 67.47% <68.29%> (-0.67%) ⬇️

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.

@getvictor
Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 23, 2026

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 23, 2026

Walkthrough

This PR fixes dead rows accumulating in software host counts tables by implementing an atomic table swap strategy during synchronization instead of in-place updates. A new migration cleans up existing zero-count rows and adds CHECK constraints (hosts_count > 0) to both software_host_counts and software_titles_host_counts tables. The SyncHostsSoftware and SyncHostsSoftwareTitles functions are refactored to populate a temporary swap table and atomically rename it into place. Query code paths are updated to remove explicit hosts_count > 0 filters since the database now enforces positive counts. Test expectations are adjusted to reflect reduced row counts.

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning PR description is incomplete. Multiple required sections from the template are missing or unchecked. Complete the following unchecked items: Input data validation, SQL injection prevention, endpoint backwards compatibility check, automated test simulation for host isolation, Database migration timestamp checks, collation verification, and any GitOps/fleetd compatibility checks as applicable.
Out of Scope Changes check ❓ Inconclusive The removal of hosts_count > 0 filters from query logic appears aligned with the issue objectives, but the removal of explicit filtering conditions requires verification that query behavior and filtering still work as intended. Verify that removing hosts_count > 0 filters and replacing with other conditions (e.g., software_title_id IS NOT NULL) maintains correct query semantics and isolation between teams/hosts.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: fixing dead rows accumulating in software host counts tables, which aligns with the primary objective from issue #35805.
Linked Issues check ✅ Passed The code changes implement both cleanup and atomic swap approaches from issue #35805: migration removes zero-count rows, CHECK constraints prevent future zeros, and SyncHostsSoftware/SyncHostsSoftwareTitles use atomic table swaps instead of in-place updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch victor/35805-zero-host-counts

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 and usage tips.

@getvictor getvictor marked this pull request as ready for review February 23, 2026 21:09
@getvictor getvictor requested a review from a team as a code owner February 23, 2026 21:09
@mostlikelee
Copy link
Copy Markdown
Contributor

LGTM, but looks like there are some conflicts @getvictor

…t-counts

# Conflicts:
#	server/datastore/mysql/schema.sql
#	server/datastore/mysql/software_titles.go
#	server/datastore/mysql/testdata/select_software_titles_sql_fixture.gz
@getvictor getvictor merged commit 6110e3d into main Feb 24, 2026
34 of 37 checks passed
@getvictor getvictor deleted the victor/35805-zero-host-counts branch February 24, 2026 21:35
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.

dead rows accumulate in software_host_counts

3 participants