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. -- Michael
signature.asc
Description: PGP signature