On Thu, Jan 11, 2018 at 8:14 AM, Michael Paquier <michael.paqu...@gmail.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
