Re: BUG #14941: Vacuum crashes - Mailing list pgsql-hackers

On Tue, Dec 19, 2017 at 2:35 AM, Bossart, Nathan <bossartn@amazon.com> wrote:
> On 12/11/17, 9:41 PM, "Masahiko Sawada" <sawada.mshk@gmail.com> wrote:
>> For 0003 patch, if we do VACUUM (NOWAIT) to whole database we don't
>> get the above WARNING messages, but I think we should get them. The
>> reported issue also did VACUUM FULL and REINDEX to whole database.
>> Especially when VACUUM to whole database the information of skipped
>> relations would be useful for users.
>
> Here is a new set of patches.  0001 and 0002 are essentially the same
> as before except for a rebase and some small indentation fixes.  0003
> now includes logic to log all skipped relations regardless of whether
> they were specified in the command.  I've also modified the isolation
> test slightly to use partitioned tables instead of running VACUUM and
> ANALYZE on the whole database.  This helps prevent timeouts on build-
> farm machines with CLOBBER_CACHE_ALWAYS enabled (like with 0a3edbb3).
>

Thank you for updating the patches.

According to the following old comment, there might be reason why we
didn't pass the information to vacuum_rel(). But your patch fetches
the relation
name even if the "relation" is not provided. I wonder if it can be
problem in some cases.

-                * If the RangeVar is not defined, we do not have
enough information
-                * to provide a meaningful log statement.  Chances are that
-                * vacuum_rel's caller has intentionally not provided
this information
-                * so that this logging is skipped, anyway.
+                * If relname is NULL, we do not have enough
information to provide a
+                * meaningful log statement.  Chances are that this
information was
+                * intentionally not provided so that this logging is
skipped, anyway.


It would be an another discussion but autovacuum logs usually use
explicit names. Since the following two logs could be emitted by
autovacuum I wonder if we can make an explicit relation name if we're
autovacuum.

     if (elevel != 0)
     {
         if (!rel_lock)
             ereport(elevel,
                     (errcode(ERRCODE_LOCK_NOT_AVAILABLE),
                      errmsg("skipping vacuum of \"%s\" --- lock not available",
                            relname)));
         else
             ereport(elevel,
                     (errcode(ERRCODE_UNDEFINED_TABLE),
                      errmsg("skipping vacuum of \"%s\" --- relation
no longer exists",
                            relname)));
     }

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center