Re: a verbose option for autovacuum

On Thu, Mar 18, 2021 at 9:13 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Thu, Mar 18, 2021 at 3:41 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> >
> > On Wed, Mar 17, 2021 at 08:50:26AM -0300, Euler Taveira wrote:
> > > Since commit 5f8727f5a6, this patch doesn't apply anymore. Fortunately, it is
> > > just a small hunk. I reviewed this patch and it looks good to me. There is just
> > > a small issue (double space after 'if') that I fixed in the attached version.
> >
> > No major objections to what you are proposing here.
> >
> > > /* and log the action if appropriate */
> > > - if (IsAutoVacuumWorkerProcess() && params->log_min_duration >= 0)
> > > + if (IsAutoVacuumWorkerProcess())
> > > {
> > > - TimestampTz endtime = GetCurrentTimestamp();
> > > + TimestampTz endtime = 0;
> > > + int i;
> > >
> > > - if (params->log_min_duration == 0 ||
> > > - TimestampDifferenceExceeds(starttime, endtime,
> > > - params->log_min_duration))
> > > + if (params->log_min_duration >= 0)
> > > + endtime = GetCurrentTimestamp();
> > > +
> > > + if (endtime > 0 &&
> > > + (params->log_min_duration == 0 ||
> > > + TimestampDifferenceExceeds(starttime, endtime,
> >
> > Why is there any need to actually change this part? If I am following
> > the patch correctly, the reason why you are doing things this way is
> > to free the set of N statistics all the time for autovacuum. However,
> > we could make that much simpler, and your patch is already half-way
> > through that by adding the stats of the N indexes to LVRelStats. Here
> > is the idea:
> > - Allocation of the N items for IndexBulkDeleteResult at the beginning
> > of heap_vacuum_rel(). It seems to me that we are going to need the N
> > index names within LVRelStats, to be able to still call
> > vac_close_indexes() *before* logging the stats.
> > - No need to pass down indstats for the two callers of
> > lazy_vacuum_all_indexes().
> > - Clean up of vacrelstats once heap_vacuum_rel() is done with it.
>
> Okay, I've updated the patch accordingly. If we add
> IndexBulkDeleteResult to LVRelStats I think we can remove
> IndexBulkDeleteResult function argument also from some other functions
> such as lazy_parallel_vacuum_indexes() and vacuum_indexes_leader().
> Please review the attached patch.

Sorry, I attached the wrong version patch. So attached the right one.

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/