Hi, I've pushed fixes for 1) and 2) and am working on 3).
On 2025-04-01 17:13:24 -0700, Noah Misch wrote: > On Tue, Apr 01, 2025 at 06:25:28PM -0400, Andres Freund wrote: > > On 2025-04-01 17:47:51 -0400, Andres Freund wrote: > > > 3) Some subtests fail if RELCACHE_FORCE_RELEASE and > > > CATCACHE_FORCE_RELEASE are defined: > > > > > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prion&dt=2025-04-01%2019%3A23%3A07 > > > > > > # +++ tap check in src/test/modules/test_aio +++ > > > > > > # Failed test 'worker: batch_start() leak & cleanup in implicit xact: > > > expected stderr' > > > # at t/001_aio.pl line 318. > > > # 'psql:<stdin>:4: ERROR: starting batch while batch > > > already in progress' > > > # doesn't match '(?^:open AIO batch at end)' > > > > > > > > > The problem is basically that the test intentionally forgets to exit > > > batchmode > > > - normally that would trigger an error at the end of the transaction, > > > which > > > the test verifies. However, with RELCACHE_FORCE_RELEASE and > > > CATCACHE_FORCE_RELEASE defined, we get other code entering batchmode and > > > erroring out because batchmode isn't allowed to be entered recursively. > > > > I don't really have a good idea how to deal with that yet. > > > > Hm. Making the query something like > > > > SELECT * FROM (VALUES (NULL), (batch_start())); > > > > avoids the wrong output, because the type lookup happens for the first row > > already. But that's pretty magical and probably fragile. > > Hmm. Some options: > > a. VALUES() trick above. For test code, it's hard to argue with something > that seems to solve it in practice. I think I'll go for a slightly nicer version of that, namely SELECT WHERE batch_start() IS NULL I think that ends up the least verbose of the ideas we've been discussing. > c. Move RELCACHE_FORCE_RELEASE and CATCACHE_FORCE_RELEASE to be > GUC-controlled, like how CLOBBER_CACHE_ALWAYS changed into the > debug_discard_caches GUC. Then disable them for relevant parts of > test_aio. This feels best long-term, but it's bigger. I also wanted this > in syscache-update-pruned.spec[1]. Yea, that'd probably be a good thing medium-term. Greetings, Andres Freund
