Skip to content

feat(server): prune sessions past a configurable retention window - #6888

Open
otavio wants to merge 2 commits into
perf/device-heartbeat-hot-updatesfrom
perf/session-retention-policy
Open

feat(server): prune sessions past a configurable retention window#6888
otavio wants to merge 2 commits into
perf/device-heartbeat-hot-updatesfrom
perf/session-retention-policy

Conversation

@otavio

@otavio otavio commented Aug 11, 2026

Copy link
Copy Markdown
Member

Part of the perf stack, now based on #6883#6886 left the stack and targets master on its
own. Two commits: migration 022, then the retention window itself (the follow-up fix for
head-of-line blocking is squashed into it, since the feature never shipped without it).

Closes shellhub-io/team#201 — that trailer cannot close a team issue from here, so the issue
needs closing by hand on merge.

Why

Nothing pruned sessions or session_events. On the instance this was measured on the oldest
session dated from 2023-06-05, and session_events alone 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, and
it 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_events gets no vacuum until ~1.3M
rows 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 next
acts, 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.

  • Off by default. The deletion is permanent and unattended, and 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 retention commitment and the volume
    are both known.
  • Batched, and capped at 100k sessions per run. An instance adopting this can have years to
    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.
  • Active sessions are never pruned. closed is not usable for this — a session whose server
    died 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.
  • Recordings go 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.
    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

findNonTransactionalStatement is anchored to the start of a statement. That 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. Every keyword in that
list is statement-initial, and the check is now per-statement, so it is strictly stronger.

Testing

  • Store: list/delete pair covered for cutoff, ordering, limit, active-session exclusion and
    event cascade. Mutation-checked — flipping the ORDER BY and removing the anti-join each fail.
  • Service: batching, the per-run cap, the disabled window, and that a failing recording prune
    leaves the rows alone.
  • Migration: reloptions and last_analyze asserted on a migrated database.
  • Full server suite (27 packages) and golangci-lint run ./... clean.

@otavio
otavio requested review from a team as code owners August 11, 2026 22:22
@otavio
otavio force-pushed the perf/session-retention-policy branch from 7b8205d to 6e33420 Compare August 12, 2026 16:33
@otavio
otavio force-pushed the perf/session-retention-policy branch from 6e33420 to 1848aeb Compare August 12, 2026 21:46
@otavio
otavio changed the base branch from perf/enable-pg-stat-statements to perf/device-heartbeat-hot-updates August 13, 2026 11:59
@otavio
otavio force-pushed the perf/session-retention-policy branch from 1848aeb to 3df2e69 Compare August 13, 2026 12:34
otavio added 2 commits August 13, 2026 10:28
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
otavio force-pushed the perf/session-retention-policy branch from 3df2e69 to 7f5a963 Compare August 13, 2026 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant