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
somethinglike 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
parallelleader and waits for all workers. Possible we can update stats in lazy_parallel_vacuum_or_cleanup_indexes after
WaitForParallelWorkersToFinishcall.
 

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

regards, Sergei