On 12/6/17, 11:57 AM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <rhaas@postgresql.org> 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=2w1nuSSruw@mail.gmail.com