Re: pgsql: When VACUUM or ANALYZE skips a concurrently dropped table,log i

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