Skip to content

Assert the single-backend crash blast radius (#217) - #223

Merged
jdatcmd merged 1 commit into
jdatcmd:mainfrom
ChronicallyJD:crash/217-backend-crash
Jul 29, 2026
Merged

Assert the single-backend crash blast radius (#217)#223
jdatcmd merged 1 commit into
jdatcmd:mainfrom
ChronicallyJD:crash/217-backend-crash

Conversation

@ChronicallyJD

Copy link
Copy Markdown
Collaborator

Alpha gate 4 of 4. Asserts and documents the blast radius of a single backend crash, the one you said should be a guarantee rather than an inference from the #210 repros.

The audit came first, and found no exception

The guarantee rests on there being no columnar write path that bypasses the buffer manager or WAL. I checked every write in columnar_storage.c, columnar_write_state.c, and columnar_metadata.c:

  • data pages are dirtied and WAL-logged with log_newpage_buffer,
  • the initial metapage and reserved page are log_newpaged before the smgrextend,
  • truncation is an XLOG_SMGR_TRUNCATE record.

Nothing writes the file directly and nothing leaves shared state a dead backend does not clean up. So a single backend death is no different from any other backend death in PostgreSQL, and the guarantee follows from core. That is the audit result the issue asked for: an exception would have been a defect, and there is none.

The suite

test/native_backend_crash.sh, registered next to recovery.sh in the matrix. It holds an INSERT uncommitted in an open transaction (BEGIN; INSERT ...; pg_sleep), so the kill lands before commit no matter how fast the write runs, locates the inserting backend by a marker in its query text, and SIGSEGVs that one backend while the postmaster lives. After the postmaster reinitializes it asserts against a heap mirror written the same way:

  • the postmaster survived, same pid;
  • the cluster accepts new connections;
  • committed content in the columnar table matches the mirror exactly, and the mirror is unchanged;
  • the killed insert left no visible rows and no partial row group;
  • a concurrent reader parked in pg_sleep was dropped, not only the backend that died.

The kill is repeated at two points in the write path: mid-write, and after the stripes have flushed but before the commit that never comes. 19 checks, about 8 seconds.

The first cut of this suite was wrong in my favour and the mirror caught it: a plain INSERT committed before the kill, so I was crashing after commit and calling the survivors a bug. Holding the transaction open fixed it and made the kill timing-independent, which is why correctness here does not depend on how fast the box is.

The guarantee, written down

docs/ARCHITECTURE.md gains a Crash safety section stating both crash shapes (whole cluster, and one backend while the postmaster lives), what survives and what does not, and that it rests on columnar data and its catalog being ordinary WAL-logged storage. It cites recovery.sh and native_backend_crash.sh. I put it there rather than in limitations.md because it is a mechanism statement, and because #222 is currently editing limitations.md; no overlap.

Gate

Preflight clean on all five majors (15/16/17/18/19), 0 warnings each. Full matrix green on PG18 and PG19, native_backend_crash=PASS on both, 19 checks per major.

One thing worth calling out about the suite itself, because it bit the first gate: the crash mechanism was never in doubt, but wait_up declared the cluster stable on a false positive — four quick SELECT 1s at 0.1s could all land inside one transient "accepting" blip during reinit, then the assertions fired while the postmaster was still tearing down the extra reader backend and briefly refused. It only surfaced in the with-reader scenario, and only on the two liveness-dependent checks; every durability check passed throughout. wait_up now requires a continuous 3s window (12 consecutive at 0.25s) that a reinit blip cannot fake. Verified 19/19 twice under 5x CPU contention before the gate, then green in the real 6-way matrix.

A reader can now determine, from docs/ARCHITECTURE.md alone, what a backend crash costs and what it does not, and a suite fails if that stops being true.

🤖 Generated with Claude Code

@ChronicallyJD
ChronicallyJD force-pushed the crash/217-backend-crash branch from 21a8d75 to bc17aa0 Compare July 28, 2026 23:29
@ChronicallyJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto main (8e7e97c) to clear a conflict with #225: both this branch and the parquet fuzzer register a suite on the same run_all_versions.sh line. Resolved by keeping both — native_backend_crash and fuzz_parquet sit side by side. Re-gated on the new base: preflight 5/5 zero warnings, matrix all PASS on PG18 and PG19, native_backend_crash=PASS, fuzz_parquet=PASS, harness_selftest=PASS on both. No code change, only the rebase.

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The audit is right, the guarantee is the correct one to state, and the suite runs
19/19 green here on PG17.10. One change first, and it is the same class of defect
we have both been finding all day.

18 of the 19 checks pass whether or not a backend crashed

I removed only the kill, left everything else, and ran it:

-	kill -SEGV "$pid" 2>/dev/null || true
+	: # kill removed for the vacuity proof
FAIL  a concurrent reader was dropped by the crash: got [no] want [yes]
checks run: 19

One check failed. The other eighteen passed against a run in which nothing
was ever killed.

That is not a criticism of the assertions; they are the right assertions. It is
that they are all conditional on a crash, and only the third scenario contains
anything that proves a crash happened. The mid-write and post-flush scenarios
have no such check at all:

  • postmaster survived, same pid passes trivially when nothing died.
  • committed content matches the heap mirror passes because it always did.
  • the killed insert left no visible rows passes because the insert was in an
    open transaction and the backgrounded client was killed anyway, so it rolled
    back without any crash.

So if the kill ever stops landing -- the pg_stat_activity marker changes, the
query text is rewritten, the pid lookup returns empty and kill -SEGV "" is a
silent no-op, which is exactly what my edit simulated -- two of the three
scenarios become no-ops and stay green forever. The suite would then be
asserting crash safety without ever crashing anything.

What I would like

Each scenario asserts that the crash actually occurred, not just that the
aftermath looks right. Cheapest form is the server log, which already says it:

LOG:  server process (PID nnn) was terminated by signal 11: Segmentation fault
LOG:  terminating any other active server processes

Read it incrementally from a byte offset so scenario two is not satisfied by
scenario one's crash -- that detail matters, and I got it wrong in my own fuzzer
today in the opposite direction: I re-read the log from byte zero every time and
turned one line into 257 findings. Either that, or assert the target pid is gone
and the postmaster's reinit happened, per scenario.

Then re-run the same experiment: with the kill removed, every scenario should
go red, not one.

Verified, and good

  • The audit result holds. grep -nE 'smgrwrite|FileWrite|pg_pwrite|\bwrite\('
    over columnar_storage.c, columnar_write_state.c and columnar_metadata.c
    returns nothing, and columnar_storage.c has 10 log_newpage* /
    XLOG_SMGR_TRUNCATE sites. Nothing bypasses the buffer manager, so the
    guarantee really does reduce to core's, which is the strongest form this could
    take.
  • Holding the transaction open so the kill lands before commit is the right
    design, and your note that the first cut was crashing after commit and calling
    the survivors a bug is worth keeping in the file.
  • The wait_up fix is right and the reasoning is right: four probes at 0.1s can
    all land inside one reinit blip. A continuous window is the correct shape.
  • Putting the statement in ARCHITECTURE.md rather than limitations.md is the
    right call. It is a mechanism, not a limitation, and #222 has since merged.

Fix the vacuity and I will merge it. The rest of this is ready.

Alpha gate 4 of 4. recovery.sh covers whole-cluster crash and restart. This adds
the other shape: one backend dies with SIGSEGV while the postmaster lives, which
is the jdatcmd#210 case. The postmaster reinitializes shared memory and drops every
other session, and the guarantee is that committed columnar data is intact on the
far side while an in-flight write leaves nothing.

test/native_backend_crash.sh holds an INSERT uncommitted in an open transaction
(so the kill lands before commit regardless of timing), locates the backend by a
marker in its query text, SIGSEGVs it, and after the postmaster reinitializes
asserts against a heap mirror: the postmaster survived with the same pid, the
cluster accepts new connections, committed content matches the mirror exactly,
the killed insert left no visible rows and no partial row group, and a concurrent
reader was dropped. The kill is repeated mid-write and after the stripes have
flushed but before the commit.

The audit the issue asked for found no exception: every columnar write goes
through the buffer manager and WAL (log_newpage_buffer for data pages,
log_newpage before the initial extends, XLOG_SMGR_TRUNCATE for truncation), so
the guarantee follows from core. Written into docs/ARCHITECTURE.md as a Crash
safety section, citing that mechanism and both crash shapes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ChronicallyJD
ChronicallyJD force-pushed the crash/217-backend-crash branch from bc17aa0 to 75f4132 Compare July 29, 2026 00:06
@ChronicallyJD

Copy link
Copy Markdown
Collaborator Author

Strengthened the suite after a self-review turned up two ways it could pass without proving anything, both my own "assert the premise" gap:

  1. The crash was never asserted to happen. The insert is uncommitted by design, so "no visible rows" and "content matches the mirror" are the outcome whether the SIGSEGV lands or the backend just aborts cleanly. If the signal ever missed (wrong pid, a refactor), the durability checks would pass green while testing nothing. Now every scenario opens a sentinel session before the kill and asserts the reinit dropped it — a plain backend exit drops only itself, a crash-triggered reinit drops every other session, so a surviving sentinel means no crash happened.
  2. The scenario labels were not asserted. Nothing verified the kill landed mid-INSERT vs in pg_sleep. Now the phase is read from wait_event and checked: mid-write asserts != PgSleep, post-flush polls until PgSleep then asserts, which also removes the old fixed-delay guess.

Proved the sentinel check discriminates by mutating SIGSEGV to SIGTERM (graceful exit, no reinit): the sentinel survives and the reinit check fails, where the old suite passed. That mutation also caught a bug in the check itself — waiting on a not-dropped sentinel blocked for the full sleep — now a non-blocking poll that fails fast. Connect budgets widened so the added session cannot time out under matrix contention.

19 checks -> 25. Re-gated on main (8e7e97c, includes #225): preflight 5/5 zero warnings, matrix all PASS on PG18 and PG19, native_backend_crash=PASS, harness_selftest=PASS on both. No change to the crash mechanism or the ARCHITECTURE.md section, only added assertions.

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Re-ran the same experiment rather than reading the summary. Removed only the
kill -SEGV, changed nothing else:

FAIL  mid-write: the crash reinitialized the cluster (a live session was dropped)
FAIL  post-flush: the crash reinitialized the cluster (a live session was dropped)
FAIL  a second concurrent reader was also dropped by the crash
FAIL  with-reader: the crash reinitialized the cluster (a live session was dropped)
checks run: 25

One failing check became four, one in every scenario. That is the fix, and it is
the right shape: the sentinel distinguishes a crash-triggered reinit from a
backend that merely exited, which is the only thing that separates this suite
from one that asserts nothing.

The SIGSEGV to SIGTERM mutation is a better proof than the one I asked for.
Removing the kill tests "no signal"; swapping to a graceful signal tests "the
wrong kind of death", which is the failure the sentinel is actually there to
catch, and it found a real bug in your own check while you were at it.

Asserting the phase from wait_event is worth more than the vacuity fix in the
long run. The scenario names claimed mid-write and post-flush and nothing
verified either, so a timing shift would have silently collapsed two scenarios
into one while both stayed green. Polling until PgSleep instead of guessing a
delay also removes the last timing assumption, which matters on this box.

25 checks, green as-is on PG17.10 here.

Merging. #217 can close with this.

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.

2 participants