Re: [HACKERS] Block level parallel vacuum

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: not tested

Hello

I reviewed v25 patches and have just a few notes.

missed synopsis for "PARALLEL" option (<synopsis> block in doc/src/sgml/ref/vacuum.sgml )
missed prototype for vacuum_log_cleanup_info in "non-export function prototypes"

> /*
> * Do post-vacuum cleanup, and statistics update for each index if
> * we're not in parallel lazy vacuum. If in parallel lazy vacuum, do
> * only post-vacum cleanup and update statistics at the end of parallel
> * lazy vacuum.
> */
> if (vacrelstats->useindex)
> lazy_vacuum_or_cleanup_indexes(vacrelstats, Irel, nindexes,
> indstats, lps, true);
>
> if (ParallelVacuumIsActive(lps))
> {
> /* End parallel mode and update index statistics */
> end_parallel_vacuum(lps, Irel, nindexes);
> }

I personally do not like update statistics in different places.
Can we change lazy_vacuum_or_cleanup_indexes to writing stats for both parallel and non-parallel cases? I means something like this:

> if (ParallelVacuumIsActive(lps))
> {
> /* Do parallel index vacuuming or index cleanup */
> lazy_parallel_vacuum_or_cleanup_indexes(vacrelstats, Irel,
> nindexes, stats,
> lps, for_cleanup);
> if (for_cleanup)
> {
> ...
> for (i = 0; i < nindexes; i++)
> lazy_update_index_statistics(...);
> }
> return;
> }

So all lazy_update_index_statistics would be in one place. lazy_parallel_vacuum_or_cleanup_indexes is called only from parallel leader and waits for all workers. Possible we can update stats in lazy_parallel_vacuum_or_cleanup_indexes after WaitForParallelWorkersToFinish call.

Also discussion question: vacuumdb parameters --parallel= and --jobs= will confuse users? We need more description for this options?

regards, Sergei