Horton

Re: BUG #14941: Vacuum crashes

Michael Paquier <michael.paquier@gmail.com>

View thread View on postgr.es
From: Michael Paquier <michael.paquier@gmail.com>
To: "Bossart, Nathan" <bossartn@amazon.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Lyes Ameddah <lyes.amd@gmail.com>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, "pgsql-bugs@postgresql.org" <pgsql-bugs@postgresql.org>
Date: 2018-01-09T04:27:20Z
Lists: pgsql-bugs, pgsql-hackers 
On Thu, Dec 28, 2017 at 10:46:18PM +0000, Bossart, Nathan wrote:
> I agree, this makes more sense.  I've made this change in v3 of 0003.

Based on the opinions gathered on this thread, 0001 and 0002 seem to be
in the shape wanted, and those look good for shipping. Now for 0003 we
are not there yet.

-       if (relation == NULL)
+       if (relation != NULL)
+           relname = relation->relname;
+       else if (!rel_lock)
+           relname = get_rel_name(relid);
+
+       if (relname == NULL)
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.
--
Michael

Commits

  1. Add option SKIP_LOCKED to VACUUM and ANALYZE

  2. Rename VACOPT_NOWAIT to VACOPT_SKIP_LOCKED

  3. Add parenthesized options syntax for ANALYZE.

  4. Don't allow VACUUM VERBOSE ANALYZE VERBOSE.

  5. Fix inadequate locking during get_rel_oids().

  6. Avoid having autovacuum workers wait for relation locks.