Hi Ayush, > Could we skip that scan when none of the aborting xids has catalog > changes? [...] I can prepare a patch if the direction looks right.
The direction looks right. I built your guard on current master (26bd08fb5c, which has beff3cd0098) and measured it: - Correctness: across 60,000 random transactions (DDL plus DML, nested savepoints with RELEASE, and a DML-only savepoint rolled back after the DDL), the build with your guard decoded byte-for-byte the same stream as master. I also instrumented the skip path to walk the list anyway and count what it would have removed: in 50,287 skipped aborts, 0 had any tuplecid to remove. - The loop over the subxacts is necessary, as you noted. Without it, a tuplecid_nested permutation with an INSERT at the outer savepoint (the outer subxact has no catalog changes of its own; the entries belong to a released inner subxact) fails on the cmin Assert, and the same thing crashes the differential fuzzer in every shard. With the loop, all of it passes. (ReorderBufferBuildTupleCidHash() already gates on rbtxn_has_catalog_changes() the same way; the flag is set in ReorderBufferXidSetCatalogChanges() and cleared nowhere, so it is a sound gate here too.) - Saving: it grows as (tuplecid list length) x (DML-only aborts). For short lists it is in the noise -- around 0.1 ms at 1k tuplecids and 100 aborts. It becomes real higher up: 10k/1k is 19 ms -> 3.6 ms, 50k/10k is 1067 ms -> 18 ms, and 100k/10k is 5213 ms -> 36 ms. One thing worth documenting: the guard relies on the catalog-changes flag never being cleared once set. That holds today, but a short comment or an Assert at the skip would make the dependency explicit for whoever adds a flag path later. Happy to share the extra test permutation and the fuzz/benchmark scripts if they are useful for the patch. Regards, Manu
