Skip to content

feat(postgres): enable diagnostic extensions and a deployment tuning seam - #6886

Merged
otavio merged 1 commit into
masterfrom
perf/enable-pg-stat-statements
Aug 13, 2026
Merged

feat(postgres): enable diagnostic extensions and a deployment tuning seam#6886
otavio merged 1 commit into
masterfrom
perf/enable-pg-stat-statements

Conversation

@otavio

@otavio otavio commented Aug 11, 2026

Copy link
Copy Markdown
Member

Rebased onto master, and now carries #6892 as well — the tuning work that was stacked
above this has been folded in as a single variable, so the two land together and #6892 can be
closed. Migration renumbered 021018, the next free number on master.

Table-level counters can say which table is being scanned but never which query is scanning
it. That is why the sequential-scan investigation (shellhub-io/team#200) could report 1.37 billion
sequential scans against a two-row namespaces table without being able to name a single
statement responsible. It lists enabling pg_stat_statements as its own first action.

This enables it, plus two extensions that close the neighbouring measurement gaps:

Extension Needs preload? What it answers
pg_stat_statements yes which statements, by calls and total_exec_time
pgstattuple no bloat and in-page free space directly, instead of heap-size ÷ row-count
pg_buffercache no what actually occupies shared_buffers

All three already ship in postgres:18.0, so there is no image build here — a preload flag and a
CREATE EXTENSION.

The two safety properties, both verified

1. A diagnostic must never be able to fail a boot. Migrations run inline in Server.Setup
before the listener binds and an error there is log.Fatal, and none of these three is a
trusted extension — so an unguarded CREATE EXTENSION would turn a database role without
superuser into a server that cannot start. Each statement is therefore wrapped in a DO block
that can only warn. Both halves tested against a non-superuser role:

-- bare
ERROR:  permission denied to create extension "pg_stat_statements"
HINT:  Must be superuser to create this extension.

-- guarded
NOTICE:  skipping pg_stat_statements: permission denied to create extension "pg_stat_statements"
DO

The trade-off is that a swallowed failure is not retried, since bun marks a migration applied
before running it. That is deliberate for diagnostics, and IF NOT EXISTS keeps the manual fix
idempotent.

2. The migration does not depend on the compose flag. CREATE EXTENSION pg_stat_statements succeeds without shared_preload_libraries — its install script only defines
functions and a view, and the library loads lazily. So the migration is safe on a deployment whose
postgres was started without the flag; only reading the view fails there. Verified by applying
018 against a postgres with an empty shared_preload_libraries:

=> SELECT count(*) FROM pg_stat_statements;
ERROR:  pg_stat_statements must be loaded via "shared_preload_libraries"

…while migrations completed successfully, pgstattuple worked normally, and the API kept serving.
Nothing in ShellHub queries the view, so this degraded state has no application impact.

Configuration

command: >
  postgres
  -c io_method=worker
  -c shared_preload_libraries=pg_stat_statements
  ${SHELLHUB_POSTGRES_EXTRA_ARGS:-}

Restart-only rather than reload, which an upgrade's stack recreation already provides.
compute_query_id is already at its auto default and activates itself once the library loads,
so there is no second flag.

No other GUCs, deliberately: pg_stat_statements.max (5000) is far above ShellHub's
parameterised statement count, track = top avoids nested-statement cost we have no question for,
and track_planning stays off because it is the expensive one — a contended spinlock per
plan. Cost is roughly 1 MB of shared memory and ~1% CPU.

docker-compose.postgres.test.yml is left alone: it is a standalone replacement overlay used only
by the integration-test harness, which gains nothing from statement collection.

Sizing belongs to the deployment (was #6892)

PostgreSQL otherwise runs on stock defaults everywhere — 128 MB of shared_buffers regardless of
the machine underneath, which on the largest managed instance is an 84.76% buffer cache hit ratio
against a 5.9 GB database on a 15.6 GB host (shellhub-io/team#198). The right values differ per
host, so they belong to whatever knows the host, not to this file.

What the deployment lacked was a way to say so. Compose replaces command: rather than merging
it, so an override file has to restate every flag and silently drops whatever the list grows next.
SHELLHUB_POSTGRES_EXTRA_ARGS is that seam: appended after the flags the product itself requires,
empty by default, documented in .env and set from .env.override or the managed deploy.

Appending is enough because postgres takes the final occurrence of a setting — a deployment can
raise shared_buffers or even outrank io_method without naming the flags it does not care about,
and adding a flag here later cannot break an existing override. Verified on postgres:18.0:

$ postgres -c io_method=worker -c shared_preload_libraries=pg_stat_statements \
           -c shared_buffers=256MB -c wal_compression=lz4
=> shared_buffers = 32768   -- 8 kB units, i.e. the appended 256MB won
=> wal_compression = lz4

Docker-level limits deliberately stay out of it: shm_size and mem_limit merge from an override
file the ordinary way (confirmed: 64m1g), and only command: has the replacement problem.

This carries no behaviour of its own. With the variable empty, the rendered argv is exactly
master's plus the preload flag — no warning on stderr, no stray empty argument.

It already earned its keep

Five authenticated GET /api/devices requests against the dev stack, with counters reset first:

calls statement
10 SELECT … FROM namespaces …
11 SELECT … FROM memberships …
5 SELECT … FROM users …
5 SELECT device.* … (the device list)
5 SELECT count(*) FROM devices … (its pagination count)

Five statements of identity and tenancy resolution per request, against two that do the work.
The namespace is fetched twice per request — once by GetUserRole in the authenticator, once by
ListDevices purely to read the device-limit fields — and each fetch drags in
Relation("Memberships.User"), which ListDevices never reads. That is the mechanism behind
#200's 1.37 B namespaces and ~472 M users/memberships scans, now confirmed from both ends.
Details posted on the issue; the fix is a separate change.

Testing

  • Migration guard tests and the full pg store suite green against a schema built from 001
    through 018.
  • 018 applied and reverted against a stock postgres:18.0: all three extensions created,
    pg_stat_statements readable with the preload, all three dropped cleanly on the way down.
  • docker compose config rendered with the variable empty, populated, and carrying a quoted value
    containing a space (shared_preload_libraries='pg_stat_statements, pg_prewarm' survives as one
    argument).
  • End-to-end through bun's runner on the dev stack, in both configurations (with and without the
    preload), plus the non-superuser privilege path shown above.

Refs shellhub-io/team#198, shellhub-io/team#200.

@otavio
otavio requested review from a team as code owners August 11, 2026 16:25
@gustavosbarreto
gustavosbarreto force-pushed the perf/drop-unused-session-indexes branch from f9be886 to 84ff059 Compare August 12, 2026 13:42
@gustavosbarreto
gustavosbarreto force-pushed the perf/enable-pg-stat-statements branch from 2a1352d to a1841fe Compare August 12, 2026 13:42
@otavio
otavio force-pushed the perf/drop-unused-session-indexes branch from 84ff059 to c12baa5 Compare August 12, 2026 21:39
Base automatically changed from perf/drop-unused-session-indexes to master August 12, 2026 21:39
@otavio
otavio force-pushed the perf/enable-pg-stat-statements branch from a1841fe to 43dd80a Compare August 12, 2026 21:46
@otavio
otavio changed the base branch from master to perf/device-heartbeat-hot-updates August 12, 2026 21:48
@otavio
otavio force-pushed the perf/enable-pg-stat-statements branch from 43dd80a to c9ce2ba Compare August 12, 2026 23:27
@otavio
otavio requested a review from a team as a code owner August 12, 2026 23:27
@otavio
otavio changed the base branch from perf/device-heartbeat-hot-updates to master August 12, 2026 23:29
…seam

Table-level counters can say which table is being scanned but never which query is scanning it,
which is why the sequential-scan investigation could report 1.37 billion scans on a two-row
`namespaces` table without naming a single statement. pg_stat_statements closes that gap;
pgstattuple and pg_buffercache close the neighbouring ones, reporting bloat and in-page free
space directly instead of inferring them from heap size over row count, and showing what actually
occupies shared_buffers.

All three already ship in postgres:18.0, so this is a preload flag plus a CREATE EXTENSION.
Only pg_stat_statements needs shared_preload_libraries, which takes effect on restart rather than
reload -- an upgrade recreates the stack, so it arrives with the version. compute_query_id is
already at its "auto" default and activates itself once the library loads.

Each CREATE is wrapped in a DO block that can only warn. Migrations run inline in Server.Setup
before the listener binds and an error there is fatal, and none of these three is a trusted
extension, so an unguarded CREATE EXTENSION would turn a database role without superuser into a
server that cannot boot. Verified both halves: the bare statement fails with "Must be superuser
to create this extension", the guarded one emits a notice and continues.

Creating pg_stat_statements without the preload is also safe -- its install script only defines
functions and a view, and the library loads lazily -- so the extension can be created anywhere and
merely errors on read until postgres restarts with it preloaded. Verified that too: 018 applied
cleanly against a postgres with an empty shared_preload_libraries, the view reported "must be
loaded via shared_preload_libraries", and the API served normally throughout.

The cost is roughly 1 MB of shared memory and about 1% CPU. track_planning stays off, since that
is the expensive one.

Sizing does not follow it into this file. PostgreSQL runs on stock defaults here -- 128 MB of
shared_buffers regardless of the machine underneath -- and the right values differ per host, so
they belong to the deployment that knows the host. What the deployment lacked is a way to say so:
Compose replaces `command:` rather than merging it, so an override file has to restate every flag
and silently drops whatever the list grows next.

SHELLHUB_POSTGRES_EXTRA_ARGS is that seam, appended after the flags the product itself requires.
postgres takes the final occurrence of a setting, so a deployment can raise shared_buffers or
outrank io_method without naming the flags it does not care about, and adding a flag here later
does not break an existing override. Empty by default, so nothing changes out of the box: .env
documents it with an example, and .env.override -- which bin/docker-compose already loads last --
is where a host sets it.

Docker-level limits deliberately stay out of it: shm_size and mem_limit merge from an override
file the ordinary way, and only `command:` has the wholesale-replacement problem.

Refs: shellhub-io/team#198
Refs: shellhub-io/team#200
@otavio
otavio force-pushed the perf/enable-pg-stat-statements branch from c9ce2ba to 381eb43 Compare August 13, 2026 11:30
@otavio otavio changed the title feat(postgres): enable pg_stat_statements and two measurement extensions feat(postgres): enable diagnostic extensions and a deployment tuning seam Aug 13, 2026
@otavio
otavio merged commit 62e65c3 into master Aug 13, 2026
69 checks passed
@otavio
otavio deleted the perf/enable-pg-stat-statements branch August 13, 2026 12:58
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