feat(server): prune sessions past a configurable retention window - #6888
Open
otavio wants to merge 2 commits into
Open
feat(server): prune sessions past a configurable retention window#6888otavio wants to merge 2 commits into
otavio wants to merge 2 commits into
Conversation
otavio
force-pushed
the
perf/session-retention-policy
branch
from
August 12, 2026 16:33
7b8205d to
6e33420
Compare
otavio
force-pushed
the
perf/session-retention-policy
branch
from
August 12, 2026 21:46
6e33420 to
1848aeb
Compare
otavio
changed the base branch from
perf/enable-pg-stat-statements
to
perf/device-heartbeat-hot-updates
August 13, 2026 11:59
otavio
force-pushed
the
perf/session-retention-policy
branch
from
August 13, 2026 12:34
1848aeb to
3df2e69
Compare
The default scale factors are proportions of the table, so the bigger a table gets the longer autovacuum waits. On the instance this was measured on, session_events had reached 6.7M rows and 4.5 GB, where the default 0.2 vacuum factor means no vacuum until ~1.3M rows are dead and the default 0.1 analyze factor means no analyze until ~670k changes. It had gone 45 days without one, leaving the planner estimating against stale statistics on the largest table in the database. The proportion is the wrong shape for these two tables specifically: they are the largest and the fastest growing, which is the combination the default punishes. The ALTERs only change when the daemon next acts, so the migration also runs the ANALYZE once to clear the backlog the old factor already allowed. Anchoring findNonTransactionalStatement to the start of a statement is a prerequisite, not a drive-by: unanchored, "VACUUM" matches inside autovacuum_vacuum_scale_factor and the guard condemns an ALTER TABLE that is transactional in every respect.
Nothing pruned sessions or session_events. On the instance this came from, the oldest session dated from 2023-06-05 and session_events alone was 76% of the database, which is both why it does not fit in a sane shared_buffers and a data retention gap in its own right. The window is off by default. The deletion is permanent and unattended, and on editions with recording the events are the recording, so an upgrade must not start discarding history on its own; docker-compose.enterprise.yml sets 180 days, where the commitment and the volume are known. Batched rather than one DELETE, and capped at 100k sessions per run. An instance adopting this can have years to shed, and draining that in a single night is the write storm the batching exists to avoid, on a host already producing 104 GB of WAL a day. The backlog drains over successive nights instead, which also leaves an operator who did not want this time to notice. Sessions still in active_sessions are never pruned. closed is not usable for this, since a session whose server died never gets closed and would otherwise be immortal, whereas a row in active_sessions is what a live session actually holds. Recordings are deleted before rows. A recording is an object keyed from the session UID with nothing in the schema pointing at it, so the row is the only thing that can still name it: deleting rows first would strand every object. Community has no object storage, so the pruner is a registered seam that the cloud module fills in, like the firewall and license evaluators. One unpurgeable object must not stall the rest. DeleteRecordings returns the subset it purged and the caller deletes only those rows, so a session whose recording cannot be removed holds up nothing but itself. Failing the whole batch instead would be permanent rather than transient: SessionListExpired always re-serves the oldest sessions, so a single unreachable object would abort the same batch every night and retention would stop for good, reported only by an ERROR log -- while the objects already purged in that batch kept their rows, reading as recorded but playing back nothing. A batch in which nothing can be deleted ends the run instead of re-listing the same blocked rows until the cap. SessionListExpired carries the recorded flag with each UID so only sessions that own a recording reach object storage; without it an instance that records nothing still pays a storage lookup for every session it deletes, a thousand per batch. The nil-pruner case sits behind pruneRecordings, so the cron loop does not know the seam can be absent, matching how EvaluateFirewall hides its own. Fixes: shellhub-io/team#201
otavio
force-pushed
the
perf/session-retention-policy
branch
from
August 13, 2026 13:28
3df2e69 to
7f5a963
Compare
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.
Part of the perf stack, now based on #6883 — #6886 left the stack and targets
masteron itsown. Two commits: migration
022, then the retention window itself (the follow-up fix forhead-of-line blocking is squashed into it, since the feature never shipped without it).
Closes shellhub-io/team#201 — that trailer cannot close a
teamissue from here, so the issueneeds closing by hand on merge.
Why
Nothing pruned
sessionsorsession_events. On the instance this was measured on the oldestsession dated from 2023-06-05, and
session_eventsalone was 4,517 MB across 6.7M rows — 76%of the entire database. That is why the database does not fit in a sane
shared_buffers, andit is a data retention gap in its own right (see team#106, team#96).
What
Migration 022 — autovacuum. The default scale factors are proportions of the table, so the
bigger it gets the longer autovacuum waits: at 0.2,
session_eventsgets no vacuum until ~1.3Mrows are dead, and at 0.1 no analyze until ~670k changes. It had gone 45 days without one,
leaving the planner estimating against stale statistics on the largest table in the database.
Lowered to 0.05/0.02, plus a one-off
ANALYZE— the ALTERs only change when the daemon nextacts, so they do nothing about a backlog that already exists.
Retention cron. A nightly job deletes sessions started longer ago than the window,
cascading into their events.
recording, so an upgrade must not start discarding history on its own.
docker-compose.enterprise.ymlsets 180 days, where the retention commitment and the volumeare both known.
shed; draining that in one night is the write storm the batching exists to avoid, on a host
already producing 104 GB of WAL a day. The backlog drains over successive nights instead,
which also leaves an operator who did not want this time to notice.
closedis not usable for this — a session whose serverdied never gets closed and would otherwise be immortal — so the guard is an anti-join against
active_sessions, which is what a live session actually holds.nothing in the schema pointing at it, so the row is the only thing that can still name it.
Community has no object storage, so the pruner is a registered seam (like the firewall and
license evaluators) that the cloud module fills in — see shellhub-io/cloud#2490.
Note for reviewers
findNonTransactionalStatementis anchored to the start of a statement. That is a prerequisite,not a drive-by: unanchored,
VACUUMmatches insideautovacuum_vacuum_scale_factorand theguard condemns an
ALTER TABLEthat is transactional in every respect. Every keyword in thatlist is statement-initial, and the check is now per-statement, so it is strictly stronger.
Testing
event cascade. Mutation-checked — flipping the
ORDER BYand removing the anti-join each fail.leaves the rows alone.
reloptionsandlast_analyzeasserted on a migrated database.golangci-lint run ./...clean.