On Thu, Mar 18, 2021 at 9:13 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Thu, Mar 18, 2021 at 3:41 PM Michael Paquier <mich...@paquier.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/
v4-index_stats_log.patch
Description: Binary data