Re: BUG #14941: Vacuum crashes

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