MIME-Version: 1.0
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>
 <DB3C67FCRLOO.1R5NLYCNEA6BF@gmail.com> <CAJDiXgiYiX+azuR76DcVx8fZn57m_4v6cB14-GW34mWa=qudFQ@mail.gmail.com>
 <CAD21AoDtPpkkQ_h1yf4oTx1qn4SRdTeVY3qs+9J07fYqa_4Gww@mail.gmail.com>
 <CAJDiXgi7KB7wSQ=Ux=ngdaCvJnJ5x-ehvTyiuZez+5uKHtV6iQ@mail.gmail.com>
 <CAD21AoCcHKKXsr9Oh736ejckqqS1i430xGEyJ=JP5OL0ExyP1A@mail.gmail.com>
 <CAJDiXghaFT_1sSv3q8mjyZ_RLZDgiogg0mWRvLxSWvkUi2CcLg@mail.gmail.com>
 <CAA5RZ0u63W41OmcEO+HLs4CSo-Sd3J+Q-4=04iud8V=xX4iUrA@mail.gmail.com>
 <CAJDiXgin1TXniVGJKzOTA=F9K342uVfm6O0EmubTVB=F+XSrbA@mail.gmail.com>
 <CAD21AoDadzAwibxf-+urjx=XL+eVu8=Ut-Lh2GxXUt32LbPG3Q@mail.gmail.com>
 <CAD21AoD6HhraqhOgkQJOrr0ixZkAZuqJRpzGv-B+_-ad6d5aPw@mail.gmail.com>
 <CAJDiXgiGSpqMQSOx-cVO_LtcB5GWHBy9ph7oOR4ebbX8A==kgw@mail.gmail.com>
 <CAD21AoBRRXbNJEvCjS-0XZgCEeRBzQPKmrSDjJ3wZ8TN28vaCQ@mail.gmail.com>
 <CAPpHfduBJfMcojvmYHUo8b_C=0cxRy1N+tNiNGoA3RAZq2ApaA@mail.gmail.com>
 <CAD21AoC82NeHKXc965pPUZO2eyo1U7P6cmfRJbrcPDcnd7_6hw@mail.gmail.com>
 <CAJDiXghP2kXnEz+cj3rAWNM3NdKSB_4WtnngFXpVz2omPhGr5A@mail.gmail.com>
 <CAD21AoA0bnRZC_OqKMnH-Ln+OZ9z9k56j2c_MXj8pw69O-wkBw@mail.gmail.com>
 <CAA5RZ0sSXDza7_nUUbhHL_Sws+M+HR1daKJPXHpdLuNCkwUgUg@mail.gmail.com>
 <CAJDiXggrBsbzOisf+Nu8pZkYGrpUZaFbosL1Wbm3kKxzTm4xgw@mail.gmail.com>
 <CAA5RZ0tbiPcgQEjnhdnjz6qSjfRsGrr8jGCaMcrMaoPpax3wig@mail.gmail.com>
 <CAJDiXgjt5ZmK2uvS0E8Ztt5ePYmq8Ze_dG05Zo2NUsKLHCEuYA@mail.gmail.com>
 <CAD21AoB7v5tLPXLK=qmtt6PaEC1f+Fb-gh+MwAbXfm6x4eZGNw@mail.gmail.com>
 <CAJDiXghwtUbiFnAh3nSaxTk8KFupQuMbp+g4z3wOLoQfMuqgDg@mail.gmail.com>
 <CAJDiXgjoNd4BF19HNY_FAcDUqiqsfw8cGhNOJwBxahB8P38E3Q@mail.gmail.com>
 <CAD21AoBT1LWqPZkcHpVMVh0ZOXUneO=p61t0i8cQ+kOP9qfODQ@mail.gmail.com>
 <CAJDiXggL=J0nV7PfBsMW9+UOU3KUp1jNBM9Gov1JvAX7aG_U1g@mail.gmail.com>
 <CAD21AoDz-1Zf9DOJJrdcB2=eNA4UdywthkowNp_dHmOGC-yV_g@mail.gmail.com> <CAJDiXgjzphJ313=aDwbvryHpmTi6AqE+-5crysTtzKv01-vkzA@mail.gmail.com>
In-Reply-To: <CAJDiXgjzphJ313=aDwbvryHpmTi6AqE+-5crysTtzKv01-vkzA@mail.gmail.com>
From: Sami Imseih <samimseih@gmail.com>
Date: Wed, 21 Jan 2026 16:22:11 -0600
Message-ID: <CAA5RZ0vbM_E+C0T475q2j5U1qnUgPBCPQ-3qWYdTxDCRnvE1VQ@mail.gmail.com>
Subject: Re: POC: Parallel processing of indexes in autovacuum
To: Daniil Davydov <3danissimo@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, Alexander Korotkov <aekorotkov@gmail.com>, 
	Matheus Alcantara <matheusssilv97@gmail.com>, Maxim Orlov <orlovmg@gmail.com>, 
	Postgres hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: <https://www.postgresql.org/message-id/CAA5RZ0vbM_E%2BC0T475q2j5U1qnUgPBCPQ-3qWYdTxDCRnvE1VQ%40mail.gmail.com>
Precedence: bulk

Hi,

I took a look at v20-0001,0002 and 0003 and have some comments.

v20-0001:

1/

```
+
+       /*
+        * Cap or increase number of free parallel workers according to the
+        * parameter change.
+        */
+       AutoVacuumShmem->av_freeParallelWorkers = Max(nfree_workers, 0);
+
+       /*
+        * Don't allow number of free workers to become less than zero if the
+        * patameter was decreased.
+        */
+       AutoVacuumShmem->av_freeParallelWorkers =
+               Max(AutoVacuumShmem->av_freeParallelWorkers, 0);
```

This can probably be simplified to:

```
AutoVacuumShmem->av_freeParallelWorkers = Max(nfree_workers, 0);
```

v20-0002:

1/

I don't think showing "reserved" in the logging is needed and could be
confusing.

```
parallel index vacuum/cleanup: 3 workers were planned, 3 workers were
reserved and 3 workers were launched in total
```

Also, even though the table has `autovacuum_parallel_workers = 2`, I
see 3 workers.
This is because one worker was for cleanup due to a gin index on the
table. I think it's better
to show separate lines for index vacuuming and index cleanup, just
like VACUUM VERBOSE.

```
INFO:  launched 2 parallel vacuum workers for index vacuuming (planned: 2)
INFO:  launched 1 parallel vacuum worker for index cleanup (planned: 1)
```

otherwise it will lead the user to think 3 workers were allocated for
either vacuuming or cleanup.


v20-0003:

1/

inside vacuum_delay_point, I would re-organize the checks to
first run the code block for the a/v worker:

```
if (ConfigReloadPending && AmAutoVacuumWorkerProcess())
```

then the a/v/ parallel worker:

```
if (!AmAutoVacuumWorkerProcess() && IsParallelWorker())
```

But I am also wondering if we should have a specific backend_type
for "autovacuum parallel worker" to differentiate that from the
existing "autovacuum worker".

and also we can have a helper macro like:
```
#define AmAutoVacuumParallelWorkerProcess() (MyBackendType ==
B_AUTOVAC_PARALLEL_WORKER)
```

What do you think?

2/

Add
```
+typedef struct PVSharedCostParams
````

to src/tools/pgindent/typedefs.list

3/

+               pg_atomic_init_u32(&shared->cost_params.generation, 0);
+               SpinLockInit(&shared->cost_params.spinlock);
+               pv_shared_cost_params = &(shared->cost_params);

NIT: move SpinLockInit last

4/

Instead of:

```
+       params_generation =
pg_atomic_read_u32(&pv_shared_cost_params->generation);
+
```
and then later on:
````
+       /*
+        * Increase generation of the parameters, i.e. let parallel workers know
+        * that they should re-read shared cost params.
+        */
+       pg_atomic_write_u32(&pv_shared_cost_params->generation,
+                                               params_generation + 1);
+
+       SpinLockRelease(&pv_shared_cost_params->spinlock);
```

why can't we just do:

pg_atomic_fetch_add_u32(&pv_shared_cost_params->generation, 1);

Also, do the pg_atomic_fetch_add_u32 outside of the spinlock. right?


--
Sami Imseih
Amazon Web Services (AWS)