perf(server): stop the device heartbeat from blocking HOT updates - #6883
perf(server): stop the device heartbeat from blocking HOT updates#6883otavio wants to merge 1 commit into
Conversation
|
Follow-up #6884 is stacked on this branch — it drops three unused |
|
Claude finished @otavio's task in 2m 22s —— View job Code Review CompleteReviewed 8 files across code quality, security, testing, language patterns, and architecture — no new issues found. The code looks good as-is. A few things I checked closely and found correct:
The migration ordering (drops → fillfactor → vacuum), the To request another review round, comment --- · |
ac11f9c to
8a7cda8
Compare
8a7cda8 to
e5a3a4f
Compare
a36cd9e to
6ad1d52
Compare
Every device presence heartbeat writes last_seen, and PostgreSQL disqualifies HOT whenever an indexed column changes, so devices_last_seen forced each beat to rewrite the heap tuple and insert into every index on the table. On a 58,670-device deployment that meant ~2,244 updates per second producing 107.7 GB of WAL per day, a heap bloated roughly 12x past the width of its rows, and autovacuum running continuously without ever keeping up. Dropping the index took the HOT ratio from 0.0000% to 97.25% and WAL to 31.7 GB/day, measured in production. The saving shows up in wal_records rather than wal_fpi: a non-HOT update emitted a heap record plus index-tuple inserts into two btrees, where a HOT update emits one. devices_disconnected_at goes with it. Nothing filters or orders on that column by itself, only as half of the online predicate, which is far too unselective to be worth an index scan; it served zero scans in 66 days of production counters. The index did pay for ORDER BY last_seen DESC on the device list, which now sorts over a sequential scan. That costs ~120 ms against the bloated heap but ~24 ms once compacted, which is what 022 is for. No index on last_seen can coexist with HOT here, so the read side pays instead of the write side, and the read side is the cheaper place to pay. 021 sets fillfactor because HOT eligibility is not enough: it also needs room in the page for the new tuple version, and 022 compacts away the slack the bloated heap happened to provide. Six passes over a 58,670-row table starting from a compacted heap reached 40.4% HOT and 4.7x growth at the default, against 85.1% and 3.0x at fillfactor 85 — reserving the space costs less than letting the table rediscover it by bloating. It runs before 022, which honours fillfactor as it rewrites. 022 is the first migration in the repo that cannot run inside a transaction. VACUUM is not allowed in one, and the pool speaks pgx simple protocol, where a multi-statement Exec is itself an implicit transaction block, so the file omits the .tx. suffix and keeps every statement in its own --bun:split chunk. Neither requirement is visible in the SQL, which is what TestNonTransactionalMigrations checks. Fixes: shellhub-io/team#197 Refs: shellhub-io/team#199
6ad1d52 to
ebdef97
Compare

Every device presence heartbeat writes
last_seen, and PostgreSQL disqualifies HOT whenever anindexed column changes.
devices_last_seenwas a btree on exactly that column, so no heartbeatcould ever be a HOT update — each one rewrote the heap tuple and inserted into every index on
the table.
Measured on a 58,670-device deployment: ~2,244 row updates/second, 107.7 GB of WAL per day, a
heap bloated ~12× past the width of its rows, and autovacuum running continuously without keeping
up.
What this does
020devices_last_seenanddevices_disconnected_at021ALTER TABLE devices SET (fillfactor = 85)022VACUUM (FULL, ANALYZE) devicesMigrations only — this PR no longer touches
docker-compose.postgres.yml. Thewal_compression=lz4change it used to carry is gone: #6886 adds aSHELLHUB_POSTGRES_EXTRA_ARGSseam, so a deployment that wants it appends-c wal_compression=lz4there rather than every deployment inheriting it. The measurementsbelow still include it, since that is how it was measured.
Order is load-bearing and follows from the numbering: the drops run before the rewrite so
VACUUM FULLnever rebuilds indexes that are about to disappear, and021runs before022because
VACUUM FULLhonoursfillfactoras it rewrites (the same 58,670 rows rebuild into2,257 pages at the default and 2,667 at 85).
Verified in production
Each half was applied and measured independently, then reverted:
wal_compression=lz4(not in this PR)wal_records/sThe win shows up in
wal_records, notwal_fpi: a non-HOT update emitted a heap record plusindex-tuple inserts into two btrees, where a HOT update emits one.
wal_compressionis separateand additive at ~10% — full-page images are only ~36% of WAL volume here — and it lowered
postgres CPU by 19%, because compressing a page costs less than writing the extra bytes. That
column is context for the deployment that appends the flag, not something this PR delivers: the
HOT win in the third column is independent of it.
The trade
devices_last_seendid serve the device list's defaultORDER BY last_seen DESC, which now sortsover a sequential scan: ~121 ms on the bloated heap, ~24 ms once compacted — which is why
022is part of this PR rather than a follow-up. No index onlast_seencan coexist with HOT(composite and partial alike), so this is structural: either the write side pays or the read side
does, and one endpoint at ~24 ms is much cheaper than 76 GB/day of WAL.
devices_disconnected_atgoes along for free. Nothing filters or orders on it alone, only insidethe unselective
onlinepredicate; 0 scans in 66 days of counters.Why
021is neededProduction hit 97.25% HOT with
fillfactorat the default, because a 12×-bloated heap alreadyholds all the free space HOT needs — so
fillfactoris not required for HOT to work, contraryto the original issue. It is required to keep HOT working once
022compacts that space away.Measured locally from a freshly compacted heap, six full passes over a 58,670-row table:
fillfactor = 85Better HOT and a smaller table — reserving 15% up front costs less than letting the heap
rediscover the same slack by bloating. The autovacuum scale-factor knobs proposed alongside it
showed no material effect once
fillfactoris set and are not included.022is the repo's first non-transactional migrationVACUUMcannot run inside a transaction block, so the file omits the.tx.suffix — and becausethe pool runs in pgx
QueryExecModeSimpleProtocol, where a multi-statementExecis itself animplicit transaction, every statement must sit alone between
--bun:splitmarkers. Neitherrequirement is visible in the SQL, which is what
TestNonTransactionalMigrationschecks — theguard exists for exactly this migration, and #6888 puts it in place ahead of it.
Two further edges are handled rather than ignored:
Server.Setupbefore the listener binds.lock_timeoutmakes022fail fast rather than queue behind a long snapshot (a nightly logicalbackup, say). bun marks a migration applied before running it, so a failure there costs one
crash-restart and leaves the table merely still bloated — degraded, not broken. Recover with
psql -c 'VACUUM (FULL, ANALYZE) devices;'.bun.Connthat returns to the pool without a session reset, so
SET lock_timeoutwould otherwise leakinto application queries.
VACUUM FULLneeds free space for a full copy of the table (~320 MB at the reference scale) andholds ACCESS EXCLUSIVE for its duration, which is acceptable inside the upgrade's own restart
window. Worth a line in the release notes.
Testing
TestNonTransactionalMigrations— scans every embedded migration for a statement PostgreSQLrefuses inside a transaction and asserts it neither carries
.tx.nor shares its--bun:splitchunk.TestNonTransactionalDetectioncovers the guard itself, including prosethat merely names a
VACUUM.pgstore suite green against a schema built from001through022.021applies,reloptionsbecomes{fillfactor=85},022compacts, three indexes remain, clean boot.master:go buildclean,./api/store/...green,golangci-lint run ./...reports 0 issues. The same suite is green onthe combined tree (feat(server): prune sessions past a configurable retention window #6888 merged, then this rebased on top), where migrations run
019–022with no gap.
Fixes shellhub-io/team#197.