On 12/6/17, 11:57 AM, "Tom Lane" <[email protected]> wrote: > Robert Haas <[email protected]> writes: >> When VACUUM or ANALYZE skips a concurrently dropped table, log it. > > When this went in, I was pretty skeptical of the value of an isolation > test for it, but said nothing. However, I now observe that the isolation > test is falling over on buildfarm machines with -DCLOBBER_CACHE_ALWAYS. > The buildfarm reports are a bit hard to interpret, but it's easy to > reproduce locally, and what I get is > > $ more output_iso/regression.diffs > *** > /home/postgres/pgsql/src/test/isolation/expected/vacuum-concurrent-drop.out > Mon Dec 4 17:02:55 2017 > --- > /home/postgres/pgsql/src/test/isolation/output_iso/results/vacuum-concurrent-drop.out > Wed Dec 6 12:07:37 2017 > *************** > *** 49,54 **** > --- 49,55 ---- > COMMIT; > > step analyze_all: <... completed> > + error in steps drop_and_commit analyze_all: ERROR: canceling statement due > to user request > > starting permutation: lock vac_analyze_specified drop_and_commit > step lock: > > ====================================================================== > > What appears to be happening is that a database-wide ANALYZE takes more > than a minute under CLOBBER_CACHE_ALWAYS, causing isolationtester.c's > hardwired one-minute timeout to trigger.
Thanks for digging into this. > While you could imagine doing something to get around that, I do not > believe that this test is worth memorializing in perpetuity to begin > with. I'd recommend just taking it out again. While the current version of the test is clearly broken, I thought Robert made a pretty strong argument regarding the value of the test [0]. ISTM the counter-argument is that coverage on a handful of lines of code is not worth the extra work needed to maintain the isolation test. I’m not strongly opinionated either way, but I lean towards wanting to keep the test around. Perhaps this could be fixed by modifying the database-wide cases to use partitioned tables instead. The individual partitions will not have RangeVars specified, so it would cover the case when logging should be skipped. Nathan [0] https://postgr.es/m/CA+TgmobH17W=WdduhXJhxdwHAeTazNp7MDP=k0p=2w1nuss...@mail.gmail.com
