public inbox for [email protected]
help / color / mirror / Atom feedFrom: Matheus Alcantara <[email protected]>
To: Daniil Davydov <[email protected]>
To: Masahiko Sawada <[email protected]>
Cc: Sami Imseih <[email protected]>
Cc: Maxim Orlov <[email protected]>
Cc: Postgres hackers <[email protected]>
Subject: Re: POC: Parallel processing of indexes in autovacuum
Date: Fri, 04 Jul 2025 11:21:52 -0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAJDiXghmsbTmnm--9B5bbuZXa1OL7SZ0HYppX3tx9XsdwfJBhA@mail.gmail.com>
References: <CACG=ezZOrNsuLoETLD1gAswZMuH2nGGq7Ogcc0QOE5hhWaw=cw@mail.gmail.com>
<CAD21AoCdx5ZNS_cO7bYz1Zfb+Kw1kuJV2wtewrz7T1pPpjcWGw@mail.gmail.com>
<CAJDiXgi6ZQOoSEqj9RyZMEh+HHBtmW0+PHD85UNPtKch8ubvdg@mail.gmail.com>
<CAD21AoBcoA-i-pJ_=y+jg14R8_QaJA1iwktCnu5i-C=yXDFPdA@mail.gmail.com>
<CAJDiXgjnUdE6Sk4M0unmT+9dULyFAxcum2txQKpWTuo4uQ_oXQ@mail.gmail.com>
<CAD21AoBTZdVR93JBo620B=MX-K8cdm3VRbjrBr_Vcpngk3AjVw@mail.gmail.com>
<CAA5RZ0vfBg=c_0Sa1Tpxv8tueeBk8C5qTf9TrxKBbXUqPc99Ag@mail.gmail.com>
<CAD21AoBgvUeWS8ZsXBahA1XdYayK6DJ6dx49d6Xpii-iH+Hrwg@mail.gmail.com>
<CAA5RZ0vF+Lr-jU1LAZWTGUjboUETk8oLvaNBbA5ozX6dau+how@mail.gmail.com>
<CAJDiXggueLSGMNRmLshbmFRfbo4jzks0W8bLDfUSRZ-61fPVEQ@mail.gmail.com>
<CAFY6G8cJ=DRTX75pOGerH6sk39dRt+7MSH+y_qppDdhPs=qdQA@mail.gmail.com>
<CAJDiXgg1t6wk9NjyMUTm1iKqM9GtdQ_wrEchBtz3xjWBZM8W8A@mail.gmail.com>
<CAD21AoAC0=Xi38RQcAO4A+vdmoXToZMoHfbS=KLT49fAOTH_gA@mail.gmail.com>
<CAJDiXgiD+AZKhJSn-FSRVQxtDLmJd95wDu4wtKniQF5==1JcjQ@mail.gmail.com>
<CAD21AoAM8KsqNhrZYJuf7odvxcTC0TumXazJc-r_wC5KnDFDPg@mail.gmail.com>
<CAJDiXghbcOC9OOj3ampxuyqXH0geggnosnrYUHGygkpss-RtxA@mail.gmail.com>
<CAD21AoAPnq0vrcGgeN++r1GoL8Kza7jaGL=TNzuBn6+MkR=rUQ@mail.gmail.com>
<CAJDiXghmsbTmnm--9B5bbuZXa1OL7SZ0HYppX3tx9XsdwfJBhA@mail.gmail.com>
On Wed Jun 18, 2025 at 5:03 AM -03, Daniil Davydov wrote:
>
> Thanks for the review! Please, see v5 patch :
> 1) GUC variable and field in autovacuum shmem are renamed
> 2) ParallelAutoVacuumReleaseWorkers call moved from parallel.c to
> vacuumparallel.c
> 3) max_parallel_autovacuum_workers is now PGC_SIGHUP parameter
> 4) Fix little bug (ParallelAutoVacuumReleaseWorkers in autovacuum.c:735)
>
Thanks for the new version!
The "autovacuum_max_parallel_workers" declared on guc_tables.c mention
that is capped by "max_worker_process":
+ {
+ {"autovacuum_max_parallel_workers", PGC_SIGHUP, VACUUM_AUTOVACUUM,
+ gettext_noop("Maximum number of parallel autovacuum workers, that can be taken from bgworkers pool."),
+ gettext_noop("This parameter is capped by \"max_worker_processes\" (not by \"autovacuum_max_workers\"!)."),
+ },
+ &autovacuum_max_parallel_workers,
+ 0, 0, MAX_BACKENDS,
+ check_autovacuum_max_parallel_workers, NULL, NULL
+ },
But the postgresql.conf.sample say that it is limited by
max_parallel_workers:
+#autovacuum_max_parallel_workers = 0 # disabled by default and limited by max_parallel_workers
IIUC the code, it cap by "max_worker_process", but Masahiko has mention
on [1] that it should be capped by max_parallel_workers.
---
We actually capping the autovacuum_max_parallel_workers by
max_worker_process-1, so we can't have 10 max_worker_process and 10
autovacuum_max_parallel_workers. Is that correct?
+bool
+check_autovacuum_max_parallel_workers(int *newval, void **extra,
+ GucSource source)
+{
+ if (*newval >= max_worker_processes)
+ return false;
+ return true;
+}
---
Locking unnecessary the AutovacuumLock if none if the if's is true can
cause some performance issue here? I don't think that this would be a
serious problem because this code will only be called if the
configuration file is changed during the autovacuum execution right? But
I could be wrong, so just sharing my thoughts on this (still learning
about [auto]vacuum code).
+
+/*
+ * 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)
+{
+ LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
+
+ if (AutoVacuumShmem->av_available_parallel_workers >
+ autovacuum_max_parallel_workers)
+ {
+ Assert(prev_max_parallel_workers > autovacuum_max_parallel_workers);
+
Typo on "exeed"
+ /*
+ * Number of available workers must not exeed limit.
+ *
+ * Note, that if some parallel autovacuum workers are running at this
+ * moment, available workers number will not exeed limit after releasing
+ * them (see ParallelAutoVacuumReleaseWorkers).
+ */
---
I'm not seeing an usage of this macro?
+/*
+ * RelationGetParallelAutovacuumWorkers
+ * Returns the relation's parallel_autovacuum_workers reloption setting.
+ * Note multiple eval of argument!
+ */
+#define RelationGetParallelAutovacuumWorkers(relation, defaultpw) \
+ ((relation)->rd_options ? \
+ ((StdRdOptions *) (relation)->rd_options)->autovacuum.parallel_autovacuum_workers : \
+ (defaultpw))
+
---
Also pgindent is needed on some files.
---
I've made some tests and I can confirm that is working correctly for
what I can see. I think that would be to start include the documentation
changes, what do you think?
[1] https://www.postgresql.org/message-id/CAD21AoAxTkpkLtJDgrH9dXg_h%2ByzOZpOZj3B-4FjW1Mr4qEdbQ%40mail.g...
--
Matheus Alcantara
view thread (112+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: POC: Parallel processing of indexes in autovacuum
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox