Re: Additional logging for VACUUM and ANALYZE

On Thu, Oct 5, 2017 at 1:23 AM, Bossart, Nathan <bossartn(at)amazon(dot)com> wrote:
> Presently, there are a few edge cases in vacuum_rel() and analyze_rel() that I
> believe do not have sufficient logging. This was discussed a bit in the
> vacuum-multiple-relations thread [0], but it was ultimately decided that any
> logging changes should be proposed separately.

I think that I agree with that, especially now with VACUUM allowing
multiple relations. The discussion then would be how much logging we
want. WARNING looks adapted per the discussions we had on the other
thread as manual VACUUMs can now involve much more relations, even
with partitioned tables. More opinions would be welcome.

> So, the attached patch changes the existing lock contention message to be
> emitted for non-autovacuum sessions if necessary, and it adds a "skipping"
> message when a specified relation disappears before it is processed. For
> consistency, autovacuum logs are emitted at LOG, and logs for manual commands
> are emitted at WARNING. This patch also includes a minor documentation change

This is here:
250ms or longer will be logged. In addition, when this parameter is
set to any value other than <literal>-1</literal>, a message will be
- logged if an autovacuum action is skipped due to the existence of a
- conflicting lock. Enabling this parameter can be helpful
+ logged if an autovacuum action is skipped due to a
conflicting lock or a
+ concurrently dropped relation. Enabling this parameter can be helpful
in tracking autovacuum activity. This parameter can only be set in
So that looks adapted to the patch.

> and a test that exercises a bit of this functionality.

My take on those test would be to not include them. This is a lot just
to test two logging lines where the relation has been dropped.

> If this change were to be considered for back-patching, we would likely want to
> also apply Michael's RangeVar fix for partitioned tables to 10 [1]. Without
> this change, log messages for unspecified partitions will be emitted with the
> parent's RangeVar information.

Well, that's assuming that we begin logging some information for
manual VACUUMs using the specified RangeVar, something that does not
happen at the top of upstream REL_10_STABLE, but can happen if we were
to include the patch you are proposing on this thread for
REL_10_STABLE. But the latter is not going to happen. Or did you patch
your version of v10 to do so in your stuff? For v10 the ship has
already sailed, so I think that it would be better to just let it go,
and rely on v11 which has added all the facility we wanted.
--
Michael