Re: POC: Parallel processing of indexes in autovacuum - Mailing list pgsql-hackers
From | Daniil Davydov |
---|---|
Subject | Re: POC: Parallel processing of indexes in autovacuum |
Date | |
Msg-id | CAJDiXghaFT_1sSv3q8mjyZ_RLZDgiogg0mWRvLxSWvkUi2CcLg@mail.gmail.com Whole thread Raw |
In response to | Re: POC: Parallel processing of indexes in autovacuum (Masahiko Sawada <sawada.mshk@gmail.com>) |
Responses |
Re: POC: Parallel processing of indexes in autovacuum
|
List | pgsql-hackers |
Hi, On Fri, Jul 18, 2025 at 2:43 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > On Mon, Jul 14, 2025 at 3:49 AM Daniil Davydov <3danissimo@gmail.com> wrote: > > > > This log level is used only "for messages about parallel workers launched". > > I think that such logs relate more to the parallel workers module than > > autovacuum itself. Moreover, if we emit log "planned vs. launched" each > > time, it will simplify the task of selecting the optimal value of > > 'autovacuum_max_parallel_workers' parameter. What do you think? > > INFO level is normally not sent to the server log. And regarding > autovacuums, they don't write any log mentioning it started. If we > want to write planned vs. launched I think it's better to gather these > statistics during execution and write it together with other existing > logs. > > > > > About "%svacuum" - I guess we need to clarify what exactly the workers > > were launched for. I'll add errhint to this log, but I don't know whether such > > approach is acceptable. > > I'm not sure errhint is an appropriate place. If we write such > information together with other existing autovacuum logs as I > suggested above, I think we don't need to add such information to this > log message. > I thought about it for some time and came up with this idea : 1) When gathering such statistics, we need to take into account that users might not want autovacuum to log something. Thus, we should collect statistics in "higher" level that knows about log_min_duration. 2) By analogy with the rest of the statistics, we can only accumulate a total number of planned and launched parallel workers. Alternatively, we could build an array (one element for each index scan) of "planned vs. launched". But it will make the code "dirty", and I don't sure that this will be useful. This may be a discussion point, so I will separate it to another .patch file. > I've reviewed v7 patch and here are some comments: > > + { > + { > + "parallel_autovacuum_workers", > + "Maximum number of parallel autovacuum workers that can be > taken from bgworkers pool for processing this table. " > + "If value is 0 then parallel degree will computed based on > number of indexes.", > + RELOPT_KIND_HEAP, > + ShareUpdateExclusiveLock > + }, > + -1, -1, 1024 > + }, > > Many autovacuum related reloptions have the prefix "autovacuum". So > how about renaming it to autovacuum_parallel_worker (change > check_parallel_av_gucs() name too accordingly)? > I have no objections. > --- > +bool > +check_autovacuum_max_parallel_workers(int *newval, void **extra, > + GucSource source) > +{ > + if (*newval >= max_worker_processes) > + return false; > + return true; > +} > > I think we don't need to strictly check the > autovacuum_max_parallel_workers value. Instead, we can accept any > integer value but internally cap by max_worker_processes. > I don't think that such a limitation is excessive, but I don't see similar behavior in other "max_parallel_..." GUCs, so I think we can get rid of it. I'll replace the "check hook" with an "assign hook", where autovacuum_max_parallel_workers will be limited. > --- > +/* > + * Make sure that number of available parallel workers corresponds to the > + * autovacuum_max_parallel_workers parameter (after it was changed). > + */ > +static void > +check_parallel_av_gucs(int prev_max_parallel_workers) > +{ > > I think this function doesn't just check the value but does adjust the > number of available workers, so how about > adjust_free_parallel_workers() or something along these lines? I agree, it's better this way. > > --- > + /* > + * Number of available workers must not exceed limit. > + * > + * Note, that if some parallel autovacuum workers are running at this > + * moment, available workers number will not exceed limit after > + * releasing them (see ParallelAutoVacuumReleaseWorkers). > + */ > + AutoVacuumShmem->av_freeParallelWorkers = > + autovacuum_max_parallel_workers; > > I think the comment refers to the following code in > AutoVacuumReleaseParallelWorkers(): > > + /* > + * If autovacuum_max_parallel_workers variable was reduced during parallel > + * autovacuum execution, we must cap available workers number by its new > + * value. > + */ > + if (AutoVacuumShmem->av_freeParallelWorkers > > + autovacuum_max_parallel_workers) > + { > + AutoVacuumShmem->av_freeParallelWorkers = > + autovacuum_max_parallel_workers; > + } > > After the autovacuum launchers decreases av_freeParallelWorkers, it's > not guaranteed that the autovacuum worker already reflects the new > value from the config file when executing the > AutoVacuumReleaseParallelWorkers(), which leds to skips the above > codes. For example, suppose that autovacuum_max_parallel_workers is 10 > and 3 parallel workers are running by one autovacuum worker (i.e., > av_freeParallelWorkers = 7 now), if the user changes > autovacuum_max_parallel_workers to 5, the autovacuum launchers adjust > av_freeParallelWorkers to 5. However, if the worker doesn't reload the > config file and executes AutoVacuumReleaseParallelWorkers(), it > increases av_freeParallelWorkers to 8 and skips the adjusting logic. > I've not tested this scenarios so I might be missing something though. > Yes, this is a possible scenario. I'll rework av_freeParallelWorkers calculation. Main change is that a/v worker now checks whether config reload is pending. Thus, it will have the relevant value of the autovacuum_max_parallel_workers parameter. Thank you very much for your comments! Please, see v8 patches: 1) Rename table option. 2) Replace check_hook with assign_hook for autovacuum_max_parallel_workers. 3) Simplify and correct logic for handling autovacuum_max_parallel_workers parameter change. 4) Rework logic with "planned vs. launched" statistics for autovacuum (see second patch file). 5) Get rid of "sandbox" - I don't see the point in continuing to drag him along. -- Best regards, Daniil Davydov
Attachment
pgsql-hackers by date: