| From: | Manu <manuelreyesbravo(at)gmail(dot)com> |
|---|---|
| To: | Greg Burd <greg(at)burd(dot)me> |
| Cc: | Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, shihao zhong <zhong950419(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: UNDO with FILEOPS, and the FLUX table AM |
| Date: | 2026-10-14 02:55:18 |
| Message-ID: | 179194651883.323380.12775685554207949511@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Greg,
> Attached is v4 which I hope fixes the various cf-bot build failures.
It clears the ones I hit. I measured v4 on master b28a2b2bfa5, against v2 for
the FILEOPS cost, and checked the per-prefix test claim. Two findings, plus an
adversarial pass that came up clean.
1. ROLLBACK and crash recovery now pay two fsyncs per FILEOPS undo record
fileops_sync_inverse() fsyncs the file and its parent after every applied
inverse, and since FILEOPS can no longer be deferred that path runs inline at
ROLLBACK and in the crash-recovery undo phase. N inverses on one file is 2N
fsyncs. For N chmods in one transaction (ROLLBACK timed, median of 3), the
rollback goes from 2.5 ms on v2 to 660 ms on v4 at N=1000, and from 77 ms to
22.8 s at N=32000. Crash-recovery undo of N in-flight records, from the
server's own log: N=8000 is about 7 ms on v2 and about 5.6 s on v4. With
fsync=off, v4 at N=1000 drops from 638 ms to 1.4 ms, so the cost is entirely
the fsyncs, not asserts; the forward direction (doing the N chmods) is
unchanged from v2.
Durability of the inverse before the CLR is the point, but it can be batched:
fsync each distinct path (and its parent) once per chain, before the record
that marks the chain done, instead of once per record. As it stands a
transaction that touched one file 10k times takes about 7 s to roll back and
adds about 7 s of undo before the cluster opens after a crash.
2. "every prefix builds and passes its own tests" does not quite hold
All of 0001-0008 build without warnings and pass make check. Their own module
tests do not all run at their prefix:
- 0002: test_fileops/001 sets max_logical_revert_workers, a GUC added in 0005,
so the server will not start -- the same shape as the split issue Shihao
reported. test_fileops is not listed in src/test/modules until 0005, so the
build farm does not run it there.
- 0004: six recovery tests need the flux AM (0019) or test_idxundo (0006).
- 0005: eight test_idxundo TAP files ship here but the module arrives in 0006.
And test_fileops/016 surfaces a real issue at this prefix: with an injected
fsync failure on an inverse during abort, the backend raises ERROR inside
abort, re-enters abort five times, and ends in PANIC: ERRORDATA_STACK_SIZE
exceeded; the PANIC elevel that makes it one clean failure arrives in 0007.
On the full series, 016 passes.
- 0008: recovery 094 and 121 need the flux AM.
The full list with the exact files is in the attached notes.
3. Adversarial pass: nothing to report
On an ASan+UBSan build I ran crash injection across plain ROLLBACK, savepoints,
postmaster and backend kills, a kill during ROLLBACK, a crash inside the
recovery undo phase, and 2PC. Each time the file tree came back exactly to the
committed baseline, with no sanitizer report and no assert (a COMMIT control
diverges, so the check is live).
Repros and the full measurements are attached.
Regards,
Manu
| Attachment | Content-Type | Size |
|---|---|---|
| EVIDENCE.txt | text/plain | 7.6 KB |
| repro_rollback_fsync.sql.txt | text/plain | 761 bytes |
| repro_prefix_tests.sh.txt | text/plain | 1.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-10-14 03:14:28 | Re: [PATCH] Clear walsender reader pointers after cleanup |
| Previous Message | Chao Li | 2026-10-14 02:39:24 | Re: Add missing FreeDir in CheckTablespaceDirectory |