Re: parallel vacuum comments

On Thu, Nov 11, 2021 at 8:11 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> I've attached a draft patch that refactors parallel vacuum and
> separates parallel-vacuum-related code to new file vacuumparallel.c.
> After discussion, I'll divide the patch into logical chunks.
>
> What I'm not convinced yet in this patch is that vacuum.c,
> vacuumlazy.c and vacuumparallel.c depend on the data structure to
> store dead tuples (now called VacDeadTuples, was LVDeadTuples). I
> thought that it might be better to separate it so that a table AM can
> use another type of data structure to store dead tuples. But since I
> think it may bring complexity, currently a table AM has to use
> VacDeadTuples in order to use the parallel vacuum.
>

I think it might be better to attempt doing anything to make it
generic for tableAM in a separate patch if that is required.

Few questions/comments:
=====================
1. There are three different structures PVState,
ParallelVacuumContext, and ParallelVacuumCtl to maintain the parallel
vacuum state. Can't we merge PVState and ParallelVacuumCtl? Also, I
see that most of the fields of PVState are there in
ParallelVacuumContext except for error info fields, does it makes
sense to directly use PVState instead? Also, it would be better to
write some more comments atop each structure to explain its usage.

2. In vacuum.c, the function names doesn't match the names in their
corresponding function header comments.

3.
+ INDVAC_STATUS_COMPLETED,
+} PVIndVacStatus;

The comma is not required after the last value of enum.

--
With Regards,
Amit Kapila.