feat(api-rs): retention for session_events - #1540
Open
0xAlcibiades wants to merge 3 commits into
Open
Conversation
Every event the control plane records lands in session_events, including one row per harness stdout line, and nothing ever removed them: no TTL, no partitioning, and no delete anywhere in api-rs. Rows only went via the cascade from sessions, and sessions are not deleted in normal operation, so the table grew monotonically for the life of the deployment -- order 1.1M rows and 450MB a day on a modest single node, against a 20Gi default volume. SESSION_EVENTS_RETENTION_DAYS deletes events past the window on the existing cleanup sweep. Off by default: session_events is durable history and dropping it is not something to start doing to an existing deployment unasked. Events of a queued or running execution are never deleted, whatever their age. A long-running turn's early output is still needed to replay it, and age alone does not distinguish old from still-in-use. Deletes are batched and each sweep is bounded at 100k rows. Switching retention on for the first time can face millions of rows, and one unbounded delete would hold locks and bloat the table for the duration; successive sweeps drain the backlog instead. Migration 0054 adds the created_at index the sweep needs -- the table had only (thread_key, event_id) and (execution_id, event_type), so an age-keyed delete would seq-scan the largest table in the schema on every pass.
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
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.
Closes #1508
Takes the in-process sweep rather than pg_cron. Migration 0007's pg_cron
precedent is real, but retention that depends on an extension only the bundled
paradedb image ships would not work for deployments running their own Postgres,
and this is exactly the knob such a deployment needs.
Change
SESSION_EVENTS_RETENTION_DAYS(chart:apiRs.sessionEventsRetentionDays,default
0= off) deletessession_eventspast the window on the existingcleanup sweep, so it inherits
sandboxCleanupIntervalSecsrather than adding asecond schedule.
Off by default.
session_eventsis durable history; starting to delete iton an existing deployment because a version was bumped would be the wrong
default even though the growth is real.
Three properties worth reviewing
Events of a
queuedorrunningexecution are never deleted, whatever theirage. A long-running turn's early output is still needed to replay it, and age
alone does not separate "old" from "still in use". The
left joinalso keepsevents whose execution row is gone (
status is null), so an orphaned event isretained rather than swept by accident.
Deletes are batched, and a sweep is bounded at 100k rows (5k × 20). The
first sweep after enabling retention can face millions of rows; one unbounded
delete would hold locks and bloat the table for its duration, and a sweep that
ran until done would hold the cleanup worker and its share of the connection
pool for as long. Successive sweeps drain the backlog.
Migration 0054 adds an index on
created_at. The table had only(thread_key, event_id)and(execution_id, event_type), so an age-keyeddelete would seq-scan the largest table in the schema on every pass — the
opposite of what a background sweep should do.
Note
0054assumes0053is taken. If #1531 does not land first this should berenumbered down.
Testing
Two unit tests: retention off by default without disabling the worker (the
sandbox arms still need it), and the per-sweep bound. The delete itself is
database-backed and unexercised here — I had no Postgres available, so the SQL
rests on review and CI rather than a local run. Flagging that rather than
implying otherwise.
cargo fmt --all --checkandcargo clippy --all-targets -- -D warningscleanacross the three touched crates;
cargo testpasses.helm lintpasses andhelm templaterenders the value.