Re: [HACKERS] Block level parallel vacuum

From: Sergei Kornilov <sk(at)zsrv(dot)org>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Subject: Re: [HACKERS] Block level parallel vacuum
Date: 2019-07-20 16:44:53
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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


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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2019-07-20 16:59:51 Re: [PATCH] Incremental sort (was: PoC: Partial sort)
Previous Message James Coleman 2019-07-20 16:21:01 Re: [PATCH] Incremental sort (was: PoC: Partial sort)