Re: BUG #14941: Vacuum crashes

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: "Bossart, Nathan" <bossartn(at)amazon(dot)com>
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Lyes Ameddah <lyes(dot)amd(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, "pgsql-bugs(at)postgresql(dot)org" <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: BUG #14941: Vacuum crashes
Date: 2018-01-10 02:53:31
Message-ID: 20180110025331.GE336@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On Tue, Jan 09, 2018 at 09:40:50PM +0000, Bossart, Nathan wrote:
> On 1/8/18, 10:28 PM, "Michael Paquier" <michael(dot)paquier(at)gmail(dot)com> wrote:
>> I think that you are doing it wrong here. In get_all_vacuum_rels() you
>> should build a RangeVar to be reused in the context of this error
>> message, and hence you'll save an extra lookup based on the relation
>> OID here, saving from any timing issues that you have overseen as in
>> this code path a lock on the relation whose name is looked at is not
>> taken. Relying on the RangeVar being NULL to not generate any logs is
>> fine as a concept to me, but let's fill it where it is needed, and in
>> the case of this patch a VACUUM NOWAIT on the whole database is such a
>> case.
>
> I understand what you are saying here. I think there are two competing
> logging behaviors:
>
> 1. If a relation is concurrently dropped, we should skip logging if
> the relation was not specified in the original VACUUM/ANALYZE
> command [0]
> 2. If a relation is skipped because the lock is not available, we
> should never skip logging

At the end this comes back to if the relation is explicitely listed or
not in the command specified by the user..

> What do you think? I will take a deeper look into how your suggested
> approach might be achieved.

Backpedalling a bit on this point and coming back to this message from
Tom (https://www.postgresql.org/message-id/28748.1507071576%40sss.pgh.pa.us)
which you just cited. Why do we actually need to issue any WARNING
messages for unlisted relations? Contrary to what Sawada-san complained
upthread, it looks sane to me to not log anything if a relation is not
explicitely listed. So you should not get any warnings for a
database-wide VACUUM if a relation is dropped while the thing is
running, and what you proposed initially in
https://www.postgresql.org/message-id/D3FC73E2-9B1A-4DB4-8180-55F57D116B4E@amazon.com
is more simple, does not need to worry about any kind of timing issues,
and is consistent with autovacuum.
--
Michael

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Michael Paquier 2018-01-10 02:55:09 Re: BUG #14941: Vacuum crashes
Previous Message Andres Freund 2018-01-09 21:46:55 Re: BUG #14941: Vacuum crashes

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-01-10 02:55:09 Re: BUG #14941: Vacuum crashes
Previous Message Masahiko Sawada 2018-01-10 02:28:44 Re: [HACKERS] GUC for cleanup indexes threshold.