Skip to content

fix(db): earlier autovacuum on snapshots - #3368

Merged
ValentaTomas merged 5 commits into
mainfrom
snapshots-autovacuum-scale
Jul 24, 2026
Merged

ValentaTomas merged 5 commits into
mainfrom
snapshots-autovacuum-scale

Conversation

@ValentaTomas

@ValentaTomas ValentaTomas commented Jul 23, 2026 •

Copy link
Copy Markdown
Member

Applies the same per-table autovacuum override env_builds already received (20260723030000) to public.snapshots.

Why this is correct: the change is a storage-parameter update only — it alters when autovacuum schedules a pass, never what any query reads or writes. It takes a brief ShareUpdateExclusive lock (no rewrite, no downtime) and is trivially revertible with RESET.

What it improves: the default trigger fires only after dead tuples reach a fixed fraction of the table, and on a large table that means cleanup starts far too late — meanwhile every read of this table pays visibility checks against the accumulated dead row versions, so scan and lookup costs creep up between passes. This table sits on hot read paths (snapshot lookups during sandbox lifecycle operations), so keeping its dead-tuple population small keeps those paths at their baseline cost. Triggering earlier also makes each pass smaller and cheaper — frequent small vacuums instead of rare huge ones.

@cla-bot cla-bot Bot added the cla-signed label Jul 23, 2026
@cursor

cursor Bot commented Jul 23, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Storage-parameter-only change with a reversible RESET; no query or schema semantics change beyond autovacuum scheduling.

Overview
This adds a goose migration that lowers autovacuum vacuum, insert-vacuum, and analyze scale factors on public.snapshots so cleanup and stats refresh run sooner on a large, insert-heavy table used on hot snapshot read paths, matching the idea already applied to env_builds and including insert-driven vacuum where that table needs it. The down migration resets those storage parameters.

Reviewed by Cursor Bugbot for commit a6d522f. Bugbot is set up for automated code reviews on this repo. Configure here.

@codecov

codecov Bot commented Jul 23, 2026 •

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
3492 1 3491 7
View the top 1 failed test(s) by shortest run time
github.com/e2b-dev/infra/tests/integration/internal/tests/envd::TestCommandKillNextApp
Stack Traces | 260s run time
=== RUN   TestCommandKillNextApp
=== PAUSE TestCommandKillNextApp
=== CONT  TestCommandKillNextApp
Executing command /bin/bash in sandbox iyyg0oj05vacauyecquvu
    process_test.go:30: Build failed: {<nil> An internal error occurred. Please try again or contact support with the build ID. <nil>}
--- FAIL: TestCommandKillNextApp (259.71s)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@blacksmith-sh

This comment has been minimized.

@ValentaTomas

Copy link
Copy Markdown
Member Author

CI note: the ARM64 shard failures here are inherited from the goose version collision on main (two migrations sharing 20260723120000) — every DB-touching shard on every open PR fails the same way. #3369 renumbers the later one; once it lands I'll rerun the checks here, which should then be green. The migration in this PR is unrelated to the failure.

@ValentaTomas
ValentaTomas marked this pull request as ready for review July 24, 2026 03:57

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — straightforward autovacuum scheduling tweak mirroring the existing env_builds override.

What was reviewed: the single migration file (up/down), confirmed it only sets/resets autovacuum_vacuum_scale_factor and autovacuum_analyze_scale_factor storage parameters (no rewrite, no data touched), and checked it matches the pattern of the already-merged env_builds migration (20260723030000). The reported CI failures are unrelated (goose migration numbering collision on main, per author's comment / #3369).

Extended reasoning...

Overview

This PR adds a single new migration file, packages/db/migrations/20260723200000_snapshots_autovacuum_scale.sql, which lowers autovacuum_vacuum_scale_factor to 0.02 and autovacuum_analyze_scale_factor to 0.01 on public.snapshots, with a corresponding RESET in the down migration. This is a direct mirror of an already-merged migration (20260723030000) that applied the same override to env_builds.

Security risks

None. This is a PostgreSQL storage-parameter change affecting only autovacuum scheduling heuristics — it does not touch data, schema, permissions, or any application code path.

Level of scrutiny

Low. ALTER TABLE ... SET (...) on autovacuum scale factors takes a brief ShareUpdateExclusive lock, performs no table rewrite, and is trivially reversible via the provided RESET down migration. The change follows an established, already-shipped precedent (env_builds) rather than introducing a new pattern, so there's no novel design decision for a human to weigh in on.

Other factors

The CI test failures shown in the timeline (codecov/blacksmith) are pre-existing and unrelated to this migration — the PR author explained they stem from a goose migration filename collision on main (two migrations sharing timestamp 20260723120000), affecting every DB-touching CI shard across all open PRs, and are being fixed separately in #3369. No reviewer comments remain outstanding on this PR.

ValentaTomas added a commit that referenced this pull request Jul 24, 2026
…3369)

Two migrations merged today share the goose version `20260723120000`
(`set_tier_max_disk_size` from #3350 and
`rename_default_team_names_to_project` from #3334). Goose panics on
duplicate versions at collection time, so since the second merge
**every** goose invocation fails — all DB-touching test shards on every
open PR, and any deploy-time migration step.

Renumbers the later-merged one to `20260723172400` (its merge time).
This is safe precisely because of the failure mode: no goose run since
the collision could have applied it anywhere, so no environment has it
recorded under the old version; the earlier migration keeps its number
and its applied history.

Flagging as the likely root cause of ARM64 test-shard failures on
currently-open PRs (observed on #3368) and of any migration-step
failures in deploys since 17:24Z.
ValentaTomas added a commit that referenced this pull request Jul 24, 2026
Applies the per-table autovacuum override `env_builds` received
(20260723030000) to `public.envs`. Companion to #3368 (snapshots);
completes the family for the hot write-path tables.

**Why this is correct:** scheduling-only storage-parameter change — no
query semantics change, brief ShareUpdateExclusive lock, no rewrite,
revertible via `RESET`.

**What it improves:** this table backs template upserts and is probed by
several hot lookup paths, so it accumulates dead row versions
continuously under normal operation. With the default fractional
trigger, a large table can go a very long time before cleanup starts,
and until then every upsert and probe wades through dead versions — the
per-statement cost of the busiest write path degrades slowly and
invisibly. An earlier trigger keeps the dead-version population bounded,
which keeps upsert latency flat and also keeps planner statistics fresh
(the analyze factor), preventing plan-quality drift on these paths.
@ValentaTomas
ValentaTomas merged commit de837d9 into main Jul 24, 2026
42 checks passed
@ValentaTomas
ValentaTomas deleted the snapshots-autovacuum-scale branch July 24, 2026 19:20
ValentaTomas added a commit that referenced this pull request Jul 24, 2026
)

Autovacuum tuning for `public.env_build_assignments`, in the same
per-table override family as `env_builds` (20260723030000), `snapshots`
(#3368), and `envs` (#3372) — shaped for this table's write pattern:
rows are appended, never updated, and deleted only via occasional
cascades.

**Why this is correct:** scheduling-only storage-parameter change; brief
ShareUpdateExclusive lock; revertible via `RESET`. On an append-mostly
table the insert-driven threshold governs routine passes (dead tuples
barely accrue), so that is the primary knob; the ordinary dead-tuple
factor is lowered alongside it so a large cascade-delete burst triggers
cleanup promptly rather than waiting for unrelated insert volume.

**What it improves:** two freshness properties that decay between
passes. Planner statistics matter most — this table is joined by several
hot queries and its analyze cadence at defaults is extremely sparse for
its size, which risks plan-quality drift on exactly those paths; a lower
analyze factor keeps plan choices stable. Fresh visibility maps
additionally keep index-only scans cheap for the read paths that
qualify.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants