fix(recovery): snapshot logical-backup output FD authority - #256
Draft
seonghobae wants to merge 52 commits into
Draft
fix(recovery): snapshot logical-backup output FD authority#256seonghobae wants to merge 52 commits into
seonghobae wants to merge 52 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seonghobae
changed the base branch from
main
to
fix/recovery-evidence-weakref-coverage-b84f0c9
August 19, 2026 05:02
This was referenced Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Repair #255 and #265 without widening PostgreSQL logical-backup authority. Protected
main@b84f0c94154043a3473939c01bb6471de5a129aestill lacks this branch-only recovery work. This Draft remains an explicit child of dependency-root #233.Exact current stack — refreshed 2026-08-21
main:b84f0c94154043a3473939c01bb6471de5a129ae;#233@5951b7a4d779903b8924abaef2a387cae50b7f54;fix/logical-backup-fd-authority-b84f0c9;cfdfc263cb5ba146404934efc815fa8107ec952e;logical-backuprepair branch, with no second no-PR branch claiming this source/test surface.Canonical documentation remains separately owned by Draft #229. Protected main was not mutated.
Bounded authority contract
The branch snapshots caller output authority before inspection/use, requires an effective-user-owned private empty writable regular file with one link and offset zero, reopens the retained inode through
/proc/self/fdfor an independent package-owned offset, and revalidates final mode/link/owner/device/inode identity. It never widens a caller-provided read-only descriptor.The absolute
pg_dumptoken is opened non-blocking without following its final symlink and accepted only as a root-owned regular executable with at least one execute bit and without group/other write, set-user-ID, or set-group-ID authority. Execution occurs only through the retained descriptor with a restricted libpq environment and no shell.maximum_output_bytesis validated from 1 through signed-64-bit maximum, defaults to 64 GiB, and bounds provider bytes through a package-owned pipe and bounded-memory copy pump. Overrun, copy failure, provider failure, timeout, unsafe final state, or ordinary execution failure invalidates and durably rewinds retained output. Success proves only one time- and byte-bounded logical archive write; it does not prove archive validity, restore usability, physical backup, WAL continuity, PITR, RPO/RTO, HA, or DR.Test-first lineage
Earlier RED/GREEN lineage covers caller numeric-FD substitution, shared-open-file-description offset interference, foreign ownership, ownership drift, read-only capability widening, retained executable path replacement, unsafe executable ownership/mode/set-id authority, finite output bytes, pipe/thread construction, short writes, durable invalidation, exact-budget behavior, and concurrent error precedence.
Pump descriptor ownership
RED
55eee0b5290a1aeab1c3f6d05ba28fdabe530daedeterministically proved that closing a transferred pipe-read descriptor from the joining thread can close unrelated reused authority after the worker has already released it. The production repair leaves that descriptor solely owned by a successfully started worker; the caller closes it only ifThread.start()fails. Compensating commitd176b79f5d38e58f08757fef979d1d4eaaf75a0erestored an unrelated finalization expression accidentally replaced during that repair.Hosted timeout RCA
Exact-head CI
32390365112ond176b79f...timed out in all supported Python unit matrices near 41%. The deterministic cause was a test double that made packageos.closeraise without actually releasing the pipe writer, manufacturing an impossible no-release close semantic and preventing EOF. Test-only commit41b458cf58da60cda944ba9b2a5ec2c394f84972releases the descriptor before raising the diagnostic and proves all recorded package descriptors are closed. Python 3.10/3.12/3.14 unit jobs then passed.Exact-head coverage RCA and test-only repair
Fresh CI
32405343646on exact41b458cf58da60cda944ba9b2a5ec2c394f84972checked out that exact SHA. Supported Python unit matrices and PostgreSQL/container smokes passed, Ruff passed, and public docstrings were exactly 100%. The only failing job wasCoverage, docstrings, lint, and package: package coverage was 99.83%, with exactly eight uncovered statements inpostgres_logical_backup.pyat309-310,336-337,348-349, and357-358.Those eight statements are the four fail-safe
except PostgresLogicalBackupError: passbranches that preserve a process-controlBaseExceptionwhen cleanup invalidation itself fails during: output-thread start cancellation, thread-join cancellation, provider cancellation concurrent with an ordinary pump failure, or a pump-side process-control signal. The production behavior is intentional; the branch lacked realistic regressions proving cleanup failure cannot replace those higher-authority process-control signals.Current test-only commit
cfdfc263cb5ba146404934efc815fa8107ec952eadds those four deterministic regressions intests/test_postgres_logical_backup_error_precedence.py. It changes no production source and does not weaken coverage or error handling. The tests make_invalidate_output()fail while independently exercising each process-control path and require the originalBaseExceptionto remain primary.Exact current-head proof boundary
All four repository-local workflows for exact current head
cfdfc263cb5ba146404934efc815fa8107ec952eare now terminal-success:32418297335: success;32418297310: success;32418297306: success; and32418297289: success.Fresh formal review inventory remains empty and fresh inline review-thread inventory remains empty. These exact-head successes prove the branch-local test/security/release boundary only; they do not transfer across a future head or base change and do not satisfy the still-unintegrated #233 dependency or any later current-last-push review requirement.
Integration boundary — refreshed 2026-08-30
#233 is the exact predecessor and must integrate first. Its former read-only central coverage/review failure is historical RCA rather than the current evidence-capability boundary. Unchanged #233 now has fresh exact-contributor-head CI, Release Acceptance, Trivy Security Scan, Semgrep SAST, and bounded Strix evidence under protected central semantics. None of that evidence transfers to this child.
#233 remains non-passing because its active same-head OpenCode formal state is still
CHANGES_REQUESTED, that disposition has not been superseded by a genuinely fresh qualifying semantic review, and no qualifying approval exists under live governance. Mutable central heads, live-base ancestry, branch-head workflows/checks, reviews, and provider state are authoritative only in #244 and must be freshly read there; this Draft intentionally does not duplicate them. The central dependency remains read-only to this pg writer: do not mutate, rerun, review, merge, or copy workaround code from.github.After #233 integrates, reconcile/retarget this Draft against resulting protected main and reacquire every exact-final-head/current-base quality, security, coverage, package, provenance, release, review, and thread-resolution gate. Merge only with zero valid findings and the qualifying independent non-author approval required by then-live governance.
The physical-basebackup lane remains separately owned by #235/#257; evidence does not transfer between recovery paths.
Refs #204, #233, #244, #254, #255, #257, #265.