Repository navigation
fix(maintenance): restart for stores deleted under the daemon (#4099) - #4130
Conversation
A daemon held store files open after the profile was deleted and wrote 112 GB into unlinked inodes (#4099). Each full maintenance tick now compares every mounted store path with the file it mounted; a missing or replaced file enters the reset-required state naming `tracedecay daemon restart`, which doctor, daemon status and wipe --stale report.
|
There was a problem hiding this comment.
Devin Review found 2 potential issues.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| if continuation.is_none() | ||
| && let Some(registry) = administration.mounted_session_runtime_registry().await | ||
| { | ||
| record_lost_store_files(&maintenance_observations, ®istry); | ||
| } |
There was a problem hiding this comment.
🔴 Disabled retention hides deleted stores
When retention is disabled, record_lost_store_files never runs. MaintenanceCoordinator::spawn skips the entire retention loop, leaving deleted mounted stores absent from restart warnings.
Learn more
The new identity check runs only inside the retention tick. MaintenanceCoordinator::spawn returns without starting that tick when all retention policies are disabled. A disabled retention policy does not close mounted store handles. The daemon can therefore keep those handles without ever publishing a restart-required state.
Example: An operator disables session, observation, orphan, and compaction retention. After a project store is mounted and its database file is deleted, the daemon never runs an identity check or lists the restart action.
Recommended fix: Give store-identity checks an independent daemon-owned cadence that starts regardless of retention policy, and exercise the disabled-retention configuration in a production-path test.
Was this helpful? React with 👍 or 👎 to provide feedback.
| pub fn check_store_identities(&self) -> Vec<(PathBuf, StoreFileIdentityStateV1)> { | ||
| let mounted: Vec<_> = self | ||
| .ports | ||
| .lock() | ||
| .unwrap_or_else(std::sync::PoisonError::into_inner) | ||
| .iter() | ||
| .map(|(path, cached)| (path.clone(), cached.file_identity)) | ||
| .collect(); | ||
| mounted | ||
| .into_iter() | ||
| .map(|(path, identity)| { | ||
| let state = store_file_identity_state(&path, identity); | ||
| (path, state) | ||
| }) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
🔴 Projectless profile stores escape deletion checks
When no project route has been composed, check_store_identities has no entry for mounted profile stores. register_route_store_telemetry alone registers their ports, so projectless profile activity receives no restart warning after deletion.
Learn more
The identity census enumerates only the telemetry port map. register_route_store_telemetry is the sole production registrar and runs during project composition, not when the profile authority is mounted. attach_reset_required_stores can mount the profile registry for projectless initialize requests without composing a project. Such a mounted profile database has no identity record to compare, even though its handle remains open.
Example: A daemon serves only projectless initialize/status requests and mounts its profile authority. The profile database file is removed before any project is opened. The census stays empty and no restart action appears.
Recommended fix: Record identity as part of each canonical store mount, including profile and other non-project store mounts, instead of relying on route telemetry registration.
Was this helpful? React with 👍 or 👎 to provide feedback.
The writer's probe now releases a lost store's files, so the restart ScriptedAlchemy#4130 names only remounts the owners that keep the closed runtime.
Part of #4099.
In #4099 a profile was deleted under a running daemon. The daemon kept writing into the unlinked database files and filled 112 GB that no size census could see. Deleting files does not free that space while the daemon holds them open, and nothing checked a store's path after attach.
Every full maintenance tick now stats each mounted store path and compares its device and inode with the file recorded at mount. A store whose path is gone or names a different file enters the reset-required state for the life of the daemon, with
tracedecay daemon restartas its remedy. That surfaces in three places:tracedecay doctorlists it as a pending operator action naming the restart.tracedecay daemon statusandtracedecay_statuscount it among reset-required stores.tracedecay wipe --stalerefuses it and names the restart, because no wipe frees an unlinked inode.The daemon logs
store_file_identity_lostonce per store.The storage redesign (#4118, section 5) closes the lane and reopens it on the next job. That needs the shared executor. Until then a restart is the only way to release the descriptors.
Verification
daemon::maintenance::tests::a_mounted_store_deleted_under_the_daemon_requires_a_restartmounts a real store's telemetry port, deletes the file, runs the tick's identity check, and asserts the store is reset-required with the restart remedy. Mutation check: with the check dropped from the tick helper, the test fails.telemetry::store_file_identity_testscovers current, replaced (rename over the path), and missing files, and checks that a rebind through the held handle does not adopt a replacement.session_registry::maintenance::tests::a_store_file_lost_while_open_requires_a_daemon_restartcovers one entry per path, the restart remedy, and thatwipe --stalerefuses the label.tracedecay-maintenance,tracedecay-store-runtime, andtracedecay. Clippy andrustfmt_check_2024for every touched crate.