Re: BUG #14941: Vacuum crashes

On Thu, Jan 11, 2018 at 8:14 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Wed, Jan 10, 2018 at 05:26:43PM +0000, Bossart, Nathan wrote:
>> Right. I don't have a terribly strong opinion either way. I think the
>> counter-argument is that logging skipped relations might provide
>> valuable feedback to users. If I ran a database-wide VACUUM and a
>> relation was skipped due to lock contention, it might be nice to know
>> that so I can handle the relation individually.
>
> Or users may not care about getting information they don't care
> about. When running a database-wide VACUUM or ANALYZE you don't know
> what is exactly the list of relations this is working on. Getting
> information about things you don't know beforehand can be considered as
> misinformation.
>
> Perhaps others have different opinions. Sawada-san?

Hmm, I agree that v4 patch is more simple and consistent with current
behavior. The logging is not necessary if relation has been dropped
but I think that it's necessary if a relation exists but database-wide
VACUUM or ANALYZE could not take the lock on it. I can image a use
case where a user don't rely on autovacuum and do manual vacuums in a
maintenance window (per-week or per-day). In this case the log message
of skipping vacuum would be useful for user. The user might not be
interested in what is exactly the list of relations but might be
interested in the relations that were skipped to vacuum. IIUC since
autovacuum always passes a list of VacuumRelation (length is 1 and
having non-NULL RangeVar) to vacuum(), if autovacuum could not take
the lock on the relation it certainly emits that log. But with v4
patch, that log message is not emitted only if user do database-wide
VACUUM or ANALYZE and could not take the lock. On the other hand, the
downside of that we emits that log even if relations == NULL is we
emits that log even for toast tables and child tables. It would be
annoy user and might be unnecessary information. To deal with it, if
NOWAIT is specified we can fill RangeVar in get_all_vacuum_rels and
make vacuum_rel not emit "relation no longer exists" log. That way we
can emits that log by database-wide VACUUM/ANALYZE, while not emitting
for toast table and child tables. But I'm not sure it's worth to
effort for this direction (v3patch + tricks) going so far as making
such tricks. I'd like to hear opinions.

Regards,

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