Hi Mark,

Here is a v2 of the abort-time cleanup patch.  The code changes are
identical to v1; there are two test-related fixes:

- the new regression test is now also registered in
  contrib/test_decoding/meson.build -- v1 added it to the Makefile
  only, so Meson builds would not have run it;
- the test no longer hardcodes the regression database name when
  reconnecting to drop its scratch database (make and Meson use
  different names, which broke the test's final reconnect under
  Meson).

Re-verified on master (36f7330b8b2, --enable-cassert): the
test_decoding suite (21 regression + 14 isolation tests, including
tuplecid) passes under both build systems, and the core regression
suite passes (245/245).

One trade-off worth stating explicitly: ReorderBufferAbort() now
scans the toplevel transaction's tuplecids list, an O(N) walk per
aborted subtransaction.  I believe that is acceptable for a
backpatchable fix -- catalog-modifying subtransaction aborts are
rare, and each removal shortens the list.  The cleaner long-term
design (queue tuplecids on the writing subtransaction and merge at
commit) is a much larger change and should not block this fix.

The test considerations from my previous mail still apply: it is
probabilistic in principle, and it cannot observe the silent
corruption on non-assert builds.  And as before, I'm not attached to
either approach -- the goal is to get this fixed for 19.

Thanks,
Bingshuai Li

Attachment: v2-0001-Fix-stale-tuplecid-records-left-behind-by-aborted.patch
Description: v2-0001-Fix-stale-tuplecid-records-left-behind-by-aborted.patch

Reply via email to