On Tue, Jan 09, 2018 at 09:40:50PM +0000, Bossart, Nathan wrote:
> On 1/8/18, 10:28 PM, "Michael Paquier" <michael.paquier@gmail.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
