Re: UNDO with FILEOPS, and the FLUX table AM

Hi Manu,

Thank you for this review, and for the repros. Both findings were
right. v6, attached, fixes both of them, along with the cfbot failures
from v4. (I had a v5 ready for the cfbot failures but hadn't sent it
when your mail arrived, so its changes are in v6 too.)

> 1. ROLLBACK and crash recovery now pay two fsyncs per FILEOPS undo record

You were right, and batching was the right fix. A rollback now notes
the path each inverse touches, plus its directory, in a backend-local
table. It fsyncs each distinct path once, at the point where the
rollback is about to be recorded as complete:

- online abort and ROLLBACK PREPARED: once, after all of the abort
record's inverses have been applied, still before checkpoints are
let through, and still a PANIC on failure (test_fileops/016);
- abort redo: the same code, run by the standby's startup process;
- the startup undo phase and the logical revert worker: at the end of
the chain walk, before the walk's result is written to the ATM or
the entry is forgotten.

The table is created when the transaction writes its first FILEOPS
UNDO, so the abort path never allocates it. If an entry cannot be
added, that path is synced immediately instead.

test_fileops/017 counts the fsyncs through a new injection point that
reports each path it fsyncs. For 200 chmods on one file, without the
fix it sees 400 fsyncs in each of online rollback, abort redo on a
standby, and startup undo of a crash loser. With the fix it sees
exactly one fsync of the file and one of its directory in each case.
Your repro on an EC2 instance's NVMe volume (ext4, cassert build,
ROLLBACK only, median of 3):

N chmods before v6 ROLLBACK
200 521 ms 5.0 ms
1000 2533 ms 6.6 ms
8000 19365 ms 22.0 ms
32000 75999 ms 72.7 ms

The startup undo phase for a crash loser of about 5,000 inverses went
from 12.6 s to 13 ms. So your v2 point, "linear before open", still
holds, and the per-record cost is back near what you measured on v2.
What v6 adds over v2 is two fsyncs per distinct path per rollback.

> 2. "every prefix builds and passes its own tests" does not quite hold

Busted, apologies somehow I thought I'd kept that true but obviously
not.

- 0002: test_fileops was not registered until 0005. It is now
registered in 0002. t/001 there no longer sets
max_logical_revert_workers, and no longer expects the ATM, which
arrives in 0005. t/007's DROP TABLESPACE case moves to 0003, where
DROP TABLESPACE is routed through FILEOPS.

- 0004: per-backend UNDO has no writer other than FLUX. So the
engine tests that roll back FLUX tables (recovery 073-075, 083-085,
089, 096, 114, 117, 152, and test_checksums/025) now ship with FLUX
in 0019. 0004 keeps recovery/071, which exercises the engine at
that prefix. An earlier version of recovery/146 shipped in 0004 as
115, and 0019 deleted it; it no longer ships in 0004.

- 0005 / 0007: test_fileops/016 was the real problem. The hash
commit carried the FILEOPS, ATM and abort-ordering fixes, including
the PANIC for a failed inverse fsync during abort, and their tests.
All of that belongs to 0005 and is now there, together with the
fsync batching above. 0007 is hash only. test_idxundo t/002-009
ship with the module in 0006. test_fileops/005 and recovery 126
and 134 use the index_undo reloption, so they ship in 0006 as well.

- 0005 also carried the old tuple sLog, which 0019 replaced. It
allocated a fixed 1.29 GB "sLog Flat Hash Partitions" area in
shared memory at startup, with default settings. That is why
recovery/134 failed at 0005-0018. 0005 now adds
the final, on-demand sLog, and the old one and slog_num_partitions
are gone from the series.

- 0008: recovery 094 and 121 need FLUX and ship with it.

- test_idxundo/011 checks hash index UNDO, which arrives in 0007, so
it ships there.

- autoconf: src/test/recovery and test_fileops did not EXTRA_INSTALL
every module their tests load (btree_gist, xid_wraparound,
test_idxundo). The meson build installs all of contrib, which is
why only autoconf failed.

Running each prefix's tests and then load-testing the tip turned up a few
more test bugs, listed in the cover letter. It also turned up one real
problem I have not fixed yet. recovery/137 failed once under
EXEC_BACKEND with "FLUX undo redo: undo record pointer mismatch" while
replaying a FLUX DELETE after an immediate stop: the replayed undo insert
landed 136 bytes past the original. It did not recur in 68 more runs.
My guess, not yet confirmed, is the undo-log insert-pointer side of
the checkpoint race that recovery/152 describes. It is listed as a
known issue in the cover letter, and I'll chase it next.

> 3. Adversarial pass: nothing to report

Thank you for this one too. The S0 negative control is a good idea,
and I'll add one to my own harness. On the harness note: yes,
FileOpsRename is deferred to commit, so writing to the new name inside
the transaction fails. storage/file/README says so ("DESTRUCTIVE
operations are DEFERRED"), but too quietly. I'll make it more
prominent in the next version.

best.

-greg