Thread
-
Re: POC: Parallel processing of indexes in autovacuum
Daniil Davydov <3danissimo@gmail.com> — 2025-10-28T13:09:59Z
Hi, On Tue, Sep 16, 2025 at 1:50 AM Alexander Korotkov <aekorotkov@gmail.com> wrote: > > I've rebased this patchset to the current master. > That required me to move the new GUC definition to guc_parameters.dat. > Also, I adjusted typedefs.list and made pgindent. Thank you for looking into it! > > + { > + { > + "autovacuum_parallel_workers", > + "Maximum number of parallel autovacuum workers that can be used for processing this table.", > + RELOPT_KIND_HEAP, > + ShareUpdateExclusiveLock > + }, > + -1, -1, 1024 > + }, > > Should we use MAX_PARALLEL_WORKER_LIMIT instead of hard-coded 1024 here? I'm afraid that we will have to include an additional header file to do this. As far as I know, we are trying not to do so. For now, I will leave it hardcoded. > > - * Support routines for parallel vacuum execution. > + * Support routines for parallel vacuum and autovacuum execution. In the > + * future comments, the word "vacuum" will refer to both vacuum and > + * autovacuum. > > Not sure about the usage of word "future" here. > It doesn't look clear what it means. > Could we use "below" or "within this file"? Agree, fixed. > I see parallel_vacuum_process_all_indexes() have a TRY/CATCH block. > As I heard, the overhead of setting/doing jumps is platform-dependent, and > not harmless on some platforms. Therefore, can we skip TRY/CATCH block > for non-autovacuum vacuum? Possibly we could move it to AutoVacWorkerMain(), > that would save us from repeatedly setting a jump in autovacuum workers too. Good idea. I found try/catch block inside the "do_autovacuum" function that is obviously called only inside the autovacuum. I decided to move ReleaseAllWorkers call there. > > In general, I think this patchset badly lack of testing. I think it needs tap tests > checking from the logs that autovacuum has been done in parallel. Also, it > would be good to set up some injection points, and check that reserved > autovacuum parallel workers are getting released correctly in the case of errors. Some time ago I tried to write a test, but it looked very ugly. Your idea with injection points helped me to write much more reliable tests - see it in a new (v12) pack of patches. On Wed, Sep 17, 2025 at 1:31 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > On Mon, Sep 15, 2025 at 11:50 AM Alexander Korotkov > <aekorotkov@gmail.com> wrote: > > > > I see parallel_vacuum_process_all_indexes() have a TRY/CATCH block. As I heard, > > the overhead of setting/doing jumps is platform-dependent, and not harmless on some > > platforms. Therefore, can we skip TRY/CATCH block for non-autovacuum vacuum? > > Possibly we could move it to AutoVacWorkerMain(), that would save us from repeatedly > > setting a jump in autovacuum workers too. > > I wonder if using the TRY/CATCH block is not enough to ensure that > autovacuum workers release the reserved parallel workers in FATAL > cases. > That's true. I'll register "before_shmem_exit" callback for autovacuum, which will release workers if there are any reserved and if the a/v workers exits abnormally. > > IIUC the patch still has one problem in terms of reloading the > configuration parameters during parallel mode as I mentioned > before[1]. > Yep. I was happy to see that you think that config file processing is OK for autovacuum :) I'll allow it for a/v leader. I've also thought about "compute_parallel_delay". The simplest solution that I see is to move cost-based delay parameters to shared state (PVShared) and create some variables such a VacuumSharedCostBalance, so we can use them inside vacuum_delay_point. What do you think about this idea? Another approaches like a "tell parallel workers that they should reload config" looks a bit too invasive IMO. Thanks everybody for the review! Please, see v12 patches : 1) Implement tests for parallel autovacuum 2) Fix error with unreleased workers - see try/catch block in do_autovacuum and before_shmem_exit callback registration in AutoVacWorkerMain 3) Allow a/v leader to process config file (see guc.c) -- Best regards, Daniil Davydov