On Thu, Nov 11, 2021 at 6:38 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Nov 11, 2021 at 8:11 AM Masahiko Sawada <sawada.mshk@gmail.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.
You mean to refactor relation_vacuum table AM API too? Currently,
relation_vacuum API is responsible for whole vacuum operation and
there is no room for the core doing anything during vacuum. So
probably it doesn’t make sense to have a table AM API for parallel
vacuum.
>
> 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?
Agreed.
> Also, it would be better to
> write some more comments atop each structure to explain its usage.
Agreed.
>
> 2. In vacuum.c, the function names doesn't match the names in their
> corresponding function header comments.
Will fix.
>
> 3.
> + INDVAC_STATUS_COMPLETED,
> +} PVIndVacStatus;
>
> The comma is not required after the last value of enum.
Will fix.
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/