Skip to content

fix(stage_e_dispatch): stop the Stage C compute pool from killing the parent's DB connection across fork - #680

Merged
WilfordGrimley merged 1 commit into
masterfrom
fix/stage-c-fork-connection-drop
Aug 4, 2026
Merged

fix(stage_e_dispatch): stop the Stage C compute pool from killing the parent's DB connection across fork#680
WilfordGrimley merged 1 commit into
masterfrom
fix/stage-c-fork-connection-drop

Conversation

@WilfordGrimley

Copy link
Copy Markdown

Description

Root-causes and fixes the stream_full_catalog resume failure reported
2026-08-04: every launch since the PR #669/8b7f02c5 deploy died within
~15 seconds on the first query inside dispatch_micro_batch
(psycopg2.OperationalError: server closed the connection unexpectedly in
_partition_by_md5_verdict), with Postgres logging nothing at either
observed failure.

Root cause: _run_stage_c deliberately keeps the parent's Django DB
connection open across the ProcessPoolExecutor fork it constructs for the
Stage C compute pool (closing it there previously broke mark_ledger_failed
on a compute-side crash — see that construction site's own comment). That
means every forked compute worker inherits a live duplicate of the
parent's own TCP socket
, not an unused handle. _stage_c_compute_worker_init's
own docstring wrongly assumed the parent had already closed its connection
before forking, so its "close inherited connections" step called the real
BaseDatabaseWrapper.close() (psycopg2's PQfinish) on that live
duplicate — sending a wire-level Terminate message over the shared
socket, which silently ends the parent's own session too. A Terminate is a
clean, expected disconnect from Postgres's point of view, not a crash, which
is exactly why nothing was logged server-side. The parent's very next query
(_partition_by_md5_verdict, immediately after _run_stage_c returns) then
fails with "server closed the connection unexpectedly".

Fix: discard each worker's inherited connection reference directly
(conn.connection = None) instead of calling .close() on it. Django's own
connect() unconditionally resets every other piece of per-connection state
on next use, so this is sufficient for a genuinely fresh, independent
connection on the worker's own first query, without ever touching the shared
wire.

Also updates _SyncStagePoolStub and TestStageCComputeWorkerInit's own
fixture (test-only code) to snapshot/restore each connection wrapper's whole
__dict__ around the initializer call, rather than just patching out
.close() — the new discard-not-close behavior mutates wrapper state
(autocommit, in_atomic_block) via the lexicon-builder queries that run
later in the same call, which the old close()-patching neutralization never
had to account for.

Checklist

  • I have installed pre-commit and installed the hooks with pre-commit install before creating any commits.
  • I have updated any related tests for code I modified or added new tests where appropriate.
  • I have manually tested my changes as follows:
    • Added TestStageCComputeWorkerInitRealFork — the first test in
      test_stage_e_dispatch.py to exercise a real ProcessPoolExecutor
      fork (every other test in the file replaces it with
      _SyncStagePoolStub, which never forks and explicitly neutralizes this
      exact close call). Confirmed it fails against current master with the
      identical psycopg2.OperationalError: server closed the connection unexpectedly reported in production, and passes with this fix.
    • Full cardpicker/tests/test_stage_e_dispatch.py: 83 passed.
    • test_stream_full_catalog.py + test_run_pipeline.py: 143 passed.
    • Full cardpicker suite (host venv, /home/ubuntu/.venvs/mpcautofill-pilot,
      excludes test_federation_hash_tool_parity.py which needs
      federation-hash-tool/, not present outside the container): 3573
      passed, 8 skipped (named credential/live-fetch skips), 0 failed.
  • I have updated any relevant documentation or created new documentation where appropriate.

… parent's DB connection across fork

_run_stage_c deliberately keeps the parent's Django connection open across the
ProcessPoolExecutor fork (closing it there previously broke mark_ledger_failed).
That means every forked compute worker inherits a LIVE duplicate of the parent's
own TCP socket, not an unused handle. _stage_c_compute_worker_init's own
docstring wrongly assumed the parent had already closed its connection before
forking, so its "close inherited connections" step called the real
BaseDatabaseWrapper.close() (psycopg2's PQfinish) on that live duplicate --
sending a wire-level Terminate over the SHARED socket and silently ending the
parent's own session too. Postgres logs nothing (a Terminate is a clean
disconnect, not a crash), and the parent's very next query then fails with
"server closed the connection unexpectedly" -- exactly the stream_full_catalog
resume failure this fixes.

The fix discards each worker's inherited connection reference directly
(`conn.connection = None`) instead of calling close() on it. Django's own
connect() resets every other piece of per-connection state unconditionally on
next use, so this is sufficient for a genuinely fresh, independent connection
on the worker's own first query, without ever touching the shared wire.

Added TestStageCComputeWorkerInitRealFork, the first test in this file to
exercise a REAL ProcessPoolExecutor fork (every other test replaces it with
_SyncStagePoolStub, which never forks and explicitly neutralizes this exact
close call) -- confirmed it fails against current master with the identical
psycopg2.OperationalError reported in production, and passes with the fix.
Updated _SyncStagePoolStub and TestStageCComputeWorkerInit's own fixture to
snapshot/restore each connection wrapper's whole __dict__ around the
initializer call, since the new discard-not-close behavior mutates wrapper
state (autocommit, in_atomic_block) that the old close()-patching neutralization
never had to account for.
@WilfordGrimley
WilfordGrimley merged commit 9043176 into master Aug 4, 2026
14 checks passed
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