Repository navigation
perf(storage): batch multi-Session artifact purge during session retirement (O(M×N) guard scans) #4038
Description
Activity
Heads-up: this proposal is likely superseded by the retirement direction in discussion #4030. That discussion has converged on retiring the Artifact authority entirely, and its stated cutover invariants already subsume this issue ("retiring a Session must be proportional to that Session's owned records, never to all records in the workspace"). If retirement proceeds on that timeline, a batched-purge contract change to the current
ArtifactStoreis probably not worth the review investment. I'm repurposing this issue as a fallback stopgap option in case the retirement takes longer than expected — happy to close it if maintainers prefer to track everything under the retirement work.This comment was prepared with AI assistance (Kimi k3-256k).
Status check against current
main(7370f07a94), since two things moved since this was filed.Still true. The guard scan is unchanged —
preparePurgePathsUnlockedbuildsguardRecordsfrom every live record that is not a target (packages/storage/src/artifact-store.ts:714) and then runsresolveRemovalEntriesUnlockedover all of them (:723), so each purge issuesrealpath/lstatfor every other record in the store. SincepurgeSessionArtifactsis serialized behind the writer lock, M retiring Sessions still cost M full scans.No longer true. Cost 3 in the Problem section is fixed. #4716 replaced the
DELETE+ re-INSERT with change-tracked writes, andcompletePurgeUnlockednow commitswriteMetadataUnlocked({ deleteIds })(:791). The metadata side is already O(changed) per purge; what remains is the filesystem guard scan.The proposed direction needs rewriting. It builds on reusing the multi-ID purge intent, and #4808 removed
ArtifactStore's purge intent entirely. The only trace left is the comment at:789-790recording that session retirement owns the pending cleanup intent and retries on reopen. Question 2 in the body is moot as written.A smaller seam than the original proposal. The retirement coordinator already batches:
#drainCleanupdrains the whole cleanup queue and fans out withPromise.allSettled(packages/runtime-host/src/server/session-retirement-coordinator.ts:718). What forces M separate purges is thatpurgeSessionArtifactstakes onesessionId. Widening that signature to a set of Session ids gives one guard scan and one metadata commit per batch without touching the coordinator or introducing a new authority. That also answers Question 1: the coordinator makes no per-Session ordering promise today.On the "superseded by #4030" note above. #4030's stated invariant — "retiring a Session must be proportional to that Session's owned records, never to all records in the workspace" — is this issue's requirement, not a replacement for it. It still needs to hold for the store that exists now.
One incidental simplification for whoever picks this up: the exact-path loop at
:715-722comparesrecord.relativePathagainst the target paths, butartifact_records_relative_pathis a UNIQUE index, so it cannot fire. The alias check at:723-730is the one doing real work.This issue has had no human activity for 30 days and has been marked stale. It will be closed in 30 days unless someone comments.
If the issue is still current, please confirm it against the latest
mainand add any information that would help move it forward. Assigned issues and issues labelledpinnedare exempt from this policy.take
I'd like to pick this up, building on the readiness half in #6022. Proposed direction for the guard scan itself — make it two-tier so the filesystem is touched only for the purge set:
- Tier 1, in-memory (always): catch the dominant alias class with zero syscalls — compare casefolded, separator-normalized
relativePaths between the purge set and all remaining records. This deterministically catches case-insensitive-filesystem collisions (two records whose distinct paths address one physical file), which therelative_pathUNIQUE index cannot see on Windows/macOS. - Tier 2, bounded: resolve via realpath/lstat only the guard records Tier 1 flags as suspicious (plus the purge set itself, which purge already resolves). Leaf-level symlink aliasing between paths with unrelated directory names moves to write-time authority:
resolveArtifactPathalready refuses aliasing candidates at ingress, so a pairwise-aliased store can only arise from pre-guarantee stores or external tampering, and Tier 1 still bounds what Tier 2 must resolve.
Net effect: a
purge()stops paying O(all records) syscalls and pays O(purge set + flagged) instead, which composes with whatever batching semantics maintainers prefer for the three open questions above. Happy to adjust if the aliasing authority should stay fully at purge time.- Tier 1, in-memory (always): catch the dominant alias class with zero syscalls — compare casefolded, separator-normalized
Problem
Runtime Host session retirement drains a batch of M Sessions (
packages/runtime-host/src/server/session-retirement-coordinator.ts), and each Session's artifact cleanup triggers its own fully serializedArtifactStore.purge(). Each individual purge:realpath/lstat) for every non-target live record,DELETE+ re-INSERT of every record — see the companion metadata proposal (perf(storage): artifact metadata rewrites the full record table on every mutation (O(M×N) during startup) #4037)).So retiring M Sessions against a store of N records costs M full-record scans and M full-table metadata commits — O(M×N) on both the filesystem and the metadata side. On a real store with ~11.7k records and hundreds of retiring Sessions, this dominates cold start (profile data in this #4027 comment: ~90 s of the residual ~130 s).
#4031 made per-purge resolution concurrent (8-wide worker pool), which is a constant-factor improvement only — the review there identified batched purge as the converged design.
Proposed direction
A bulk purge path that accepts the whole retiring Session batch at once, reusing the existing multi-ID purge intent mechanism:
This removes the M multiplier without weakening case-insensitive or symlink-alias integrity and without adding another authority. Combined with change-tracked metadata write-back (companion proposal), a retirement batch becomes one O(N) guard scan + one O(changed) metadata commit.
Questions for maintainers
Related: #4027 (cold-start investigation), #4031 (bounded purge resolution; review point on the O(M×N) structure and this converged design).
This issue was prepared with AI assistance (Kimi k3-256k), including profiling and analysis.