Re: Improve behavior of concurrent ANALYZE/VACUUM

From: "Bossart, Nathan" <bossartn(at)amazon(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Postgres hackers <pgsql-hackers(at)postgresql(dot)org>, "horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp" <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
Subject: Re: Improve behavior of concurrent ANALYZE/VACUUM
Date: 2018-08-24 17:30:01
Message-ID: 6FA92FFF-24FD-448E-BD46-0E47E3B10742@amazon.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 8/23/18, 9:16 PM, "Michael Paquier" <michael(at)paquier(dot)xyz> wrote:
> Thanks, I have pushed the new test series, and reused it to check the
> new version of the main patch, which is attached. I have added a commit
> message and I have indented the thing.

Thanks for the new version!

> After pondering about it, I have also reworked the portion for
> partitioned tables so as the list of partitions processed is unchanged
> on HEAD, and we keep a consistent behavior compared to past versions.
> If VACUUM processing for partitioned tables was something new in 11, I
> think that we could have considered it, but changing silently something
> that people may rely on for more than one year now is not very
> appealing.

Agreed. Even though we're not fixing the issue for partitions yet,
this patch should still fix the originally reported authentication
issue (which I see is highlighted in your commit message). I think
there's still a slight behavior change with the ordering of the
"skipped" log messages in some cases, but that doesn't seem terribly
important. We might be able to work around this by storing all the
information we need for the log message in the VacuumRelation and
waiting to emit it until vacuum_rel() or analyze_rel(), but I doubt
it's worth the complexity.

Without patch:
postgres=> VACUUM parted1, parted2;
WARNING: skipping "parted1" --- only table or database owner can vacuum it
WARNING: skipping "parted1_part1" --- only table or database owner can vacuum it
WARNING: skipping "parted1_part2" --- only table or database owner can vacuum it
WARNING: skipping "parted2" --- only table or database owner can vacuum it
WARNING: skipping "parted2_part1" --- only table or database owner can vacuum it
WARNING: skipping "parted2_part2" --- only table or database owner can vacuum it
VACUUM

With patch:
postgres=> VACUUM parted1, parted2;
WARNING: skipping "parted1" --- only table or database owner can vacuum it
WARNING: skipping "parted2" --- only table or database owner can vacuum it
WARNING: skipping "parted1_part1" --- only table or database owner can vacuum it
WARNING: skipping "parted1_part2" --- only table or database owner can vacuum it
WARNING: skipping "parted2_part1" --- only table or database owner can vacuum it
WARNING: skipping "parted2_part2" --- only table or database owner can vacuum it
VACUUM

The new version of the patch applies cleanly, builds cleanly, and
'make check-world' succeeds. Also, I'm no longer able to reproduce
the authentication issue involving 'VACUUM FULL' run by non-
superusers, so it looks good to me.

Nathan

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2018-08-24 18:09:09 Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Previous Message Andres Freund 2018-08-24 17:02:29 Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)