Purge orphaned DuckDB states automatically once at startup - #2651
Merged
Pierre-Gilles merged 7 commits intoJul 13, 2026
Merged
Conversation
Deleting a device or disabling a feature history did not always clean the DuckDB states (the per-feature purge counted SQLite states since the DuckDB migration), so installations can carry states which no longer belong to any existing device feature. The Activity endpoint already filters them out defensively, but they consume disk space forever. Add a "purge orphaned DuckDB states" action in the DuckDB migration settings card: it deletes every state whose device_feature_id no longer exists, as a background job. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an idempotent, weekly batched purge for orphaned DuckDB device states. The purge is registered as a background job and device event, runs during initialization, records completion in a system variable, includes tests, and adds German, English, and French job labels. ChangesDuckDB orphan-state purge
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DeviceInitialization
participant DeviceManager
participant DeviceFeature
participant DuckDB
participant JobProgress
DeviceInitialization->>DeviceManager: Invoke orphaned-state purge
DeviceManager->>DeviceFeature: Fetch existing feature IDs
DeviceManager->>DuckDB: Query date range and delete orphaned rows weekly
DeviceManager->>JobProgress: Update slice progress
DeviceManager->>DeviceManager: Record purge completion flag
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2651 +/- ##
=======================================
Coverage 98.98% 98.99%
=======================================
Files 1056 1057 +1
Lines 21069 21116 +47
=======================================
+ Hits 20856 20903 +47
Misses 213 213 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/test/lib/device/device.purgeOrphanedDuckDbStates.test.js`:
- Around line 16-64: Add a test in the device.purgeOrphanedDuckDbStates suite
covering the no-device-features case: clear all DeviceFeature records while
retaining seeded DuckDB states, invoke Device.purgeOrphanedDuckDbStates(), and
verify the result reports every state for deletion and that no rows remain in
t_device_feature_state. Ensure setup/cleanup remains isolated from the existing
tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a973e7cb-89c2-427a-8e9d-f5dc336c09a0
📒 Files selected for processing (11)
front/src/config/i18n/de.jsonfront/src/config/i18n/en.jsonfront/src/config/i18n/fr.jsonfront/src/routes/settings/settings-system/SettingsSystemDuckDbMigration.jsxserver/api/controllers/device.controller.jsserver/api/routes.jsserver/lib/device/device.purgeOrphanedDuckDbStates.jsserver/lib/device/index.jsserver/test/controllers/device/device.controller.test.jsserver/test/lib/device/device.purgeOrphanedDuckDbStates.test.jsserver/utils/constants.js
7 tasks
Rework after discussion: instead of a manual button, the cleanup is a one-shot background job started automatically at boot, gated by a system variable (same pattern as the DuckDB migration). The variable is only set after a complete run, so if Gladys restarts mid-purge the job restarts at the next boot — deletes are idempotent. Deliberately slow, as requested: no upfront count (counting orphans over hundreds of millions of states held the read connection for 15-20 minutes during testing), the history is walked in monthly slices with a pause between each, so no DuckDB connection is ever held for long. DuckDB returns the number of deleted rows per statement, so the purged count is accumulated on the fly and reported by the job. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Terdious
changed the title
There was a problem hiding this comment.
♻️ Duplicate comments (1)
server/test/lib/device/device.purgeOrphanedDuckDbStates.test.js (1)
30-82: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStill missing test for the "no device features exist" edge case.
The branch where
featureIds.length === 0(producing an emptyorphanedClausethat deletes all DuckDB states) remains untested. This was flagged in a previous review and has not been addressed. As per coding guidelines: "Assume 100% patch coverage for server changes; test every added branch, error path, helper, and modified line."🧪 Suggested test using the existing buildDevice helper
it('should set the flag without purging anything on an empty table', async () => { // ... existing test ... }); + it('should purge all states when no device features exist', async () => { + // Remove all DeviceFeature records so featureIds is empty + await db.DeviceFeature.destroy({ where: {}, truncate: true }); + const { device, variable } = buildDevice(null); + const res = await device.purgeOrphanedDuckDbStates(); + expect(res).to.deep.equal({ + numberOfOrphanedDuckDbStatesToDelete: 5, // 2 existing + 3 orphaned = all + }); + const allStates = await db.duckDbReadConnectionAllAsync('SELECT * FROM t_device_feature_state'); + expect(allStates).to.have.lengthOf(0); + assert.calledWith(variable.setValue, SYSTEM_VARIABLE_NAMES.DUCKDB_ORPHANED_STATES_PURGED, 'true'); + });Note: ensure DeviceFeature records are restored after this test to avoid polluting subsequent suites.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/test/lib/device/device.purgeOrphanedDuckDbStates.test.js` around lines 30 - 82, Add a test for the featureIds.length === 0 branch in device.purgeOrphanedDuckDbStates, using buildDevice and a database state with no DeviceFeature records, and verify all DuckDB states are purged and the purge flag is set. Restore the DeviceFeature records in cleanup so this test does not affect subsequent tests.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@server/test/lib/device/device.purgeOrphanedDuckDbStates.test.js`:
- Around line 30-82: Add a test for the featureIds.length === 0 branch in
device.purgeOrphanedDuckDbStates, using buildDevice and a database state with no
DeviceFeature records, and verify all DuckDB states are purged and the purge
flag is set. Restore the DeviceFeature records in cleanup so this test does not
affect subsequent tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e4a3ef8f-6cb1-4a4b-88d2-c7d3b3af0342
📒 Files selected for processing (8)
front/src/config/i18n/de.jsonfront/src/config/i18n/en.jsonfront/src/config/i18n/fr.jsonserver/lib/device/device.init.jsserver/lib/device/device.purgeOrphanedDuckDbStates.jsserver/test/lib/device/device.init.test.jsserver/test/lib/device/device.purgeOrphanedDuckDbStates.test.jsserver/utils/constants.js
💤 Files with no reviewable changes (3)
- front/src/config/i18n/en.json
- front/src/config/i18n/de.json
- front/src/config/i18n/fr.json
Field test on a 448M-state installation: monthly slices with a fixed 100ms pause kept the CPU/disk saturated for 17 minutes — Gladys stayed up but everything was slow, and each slice held the DuckDB write connection for seconds, delaying live state processing (and scene triggers) accordingly. - Weekly slices instead of monthly: the write connection is never held more than ~1-2s. - Adaptive pause: after each slice, sleep 5x the time the slice took (capped at 60s), so the purge only ever uses ~1/6th of the resources. The one-shot purge takes a few times longer, which does not matter. - Per-slice logs with a greppable prefix (purge-orphaned-duckdb-states) showing dates, deleted count, duration and pause. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/lib/device/device.purgeOrphanedDuckDbStates.js`:
- Around line 53-68: Update the feature-snapshot and delete flow in the purge
implementation to capture a purge cutoff before snapshotting feature IDs, then
constrain every slice’s DELETE—including the final unbounded slice—to states
created no later than that cutoff. Preserve the existing slice bounds and
completion behavior, and add a regression test that creates a feature and state
after the snapshot and verifies the state survives the purge.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fac09c4c-e9db-4dd5-b4b7-7806eb9d48bb
📒 Files selected for processing (3)
server/lib/device/device.purgeOrphanedDuckDbStates.jsserver/lib/device/index.jsserver/test/lib/device/device.purgeOrphanedDuckDbStates.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
- server/lib/device/index.js
- server/test/lib/device/device.purgeOrphanedDuckDbStates.test.js
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mass deletes accumulate delete-tracking memory and WAL until the next checkpoint: flush explicitly at the end so the memory and the disk space are released right away. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Terdious
changed the title
Contributor
Author
|
Field-test results (two installations): 448M-state installation (i5 NUC-class server, shared with another Gladys + Home Assistant):
Second tester (laptop, Fedora, ~228M states): purged 45.4M orphaned states (~20% of the database!) in ~29 minutes, "no slowdown, maybe +2s on the Activity view" — deleted features and a whole device were correctly picked up by the per-feature purge (#2650) with counts displayed per job (#2652). The orphan volume found in the wild confirms the point of running this cleanup automatically for everyone. |
The feature list is snapshotted when the purge starts, but the purge runs for a long time by design: states of a feature created while it runs matched the stale NOT IN list in the (previously unbounded) last slice and were deleted. Reported by CodeRabbit. Bound every slice — including the last one — by the purge start date, and clamp the walked range to it too so even future-dated states (skewed device clocks) can never be evaluated against the stale snapshot. States orphaned after the cutoff are handled by the per-feature purge from now on, so nothing is left behind. Also add the missing test for the no-feature-left branch (empty NOT IN clause: every state is orphaned). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pierre-Gilles
approved these changes
Jul 13, 2026
7 tasks
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request check-list
To ensure your Pull Request can be accepted as fast as possible, make sure to review and check all of these items:
npm run coveragegreen locally)Did Cypress E2E tests pass?(background job, no UI)npm run eslinton front and server)npm run compare-translations— the job type label is added to en/fr/de)If your changes modify the API (REST or Node.js), did you modify the API documentation?(no API change — the earlier manual route/button of this PR was removed after discussion)If you are adding a new features/services which needs explanation, did you modify the user documentation?(automatic maintenance, visible in the jobs page)Did you add fake requests data for the demo mode?(no request)Description of change
Follow-up of #2650, reworked after the discussion on the dev channel: instead of a manual button, the cleanup of orphaned DuckDB states (states whose
device_feature_idno longer exists — leftovers of devices/features deleted while the per-feature purge counted the wrong database) is a one-shot background job started automatically at startup.DUCKDB_ORPHANED_STATES_PURGED, same pattern as the DuckDB migration), set only after a complete run: if Gladys restarts mid-purge, the job simply restarts at the next boot — deletes are idempotent, it only redoes the remaining work. No manual action ever needed.created_atslices, and after each slice the purge sleeps 5x the time the slice took (duty cycle, capped at 60s): no DuckDB connection is ever held more than ~1-2 seconds, the average CPU/disk load stays around 1/6th, and Gladys stays responsive — field-tested on a 448M-state installation where fixed short pauses saturated the CPU for 17 minutes. Per-slice logs are greppable with thepurge-orphaned-duckdb-statesprefix. The parameters are cast to UUID explicitly to avoid a per-row VARCHAR cast.Measured on a 448M-state installation: the previous count-first approach took ~30 minutes with long connection holds; the sliced walk holds each connection a few seconds at most.
🤖 Generated with Claude Code
Summary by CodeRabbit