On Thu, Nov 11, 2021 at 6:38 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Nov 11, 2021 at 8:11 AM Masahiko Sawada <sawada.m...@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/