On Tue, Nov 2, 2021 at 11:17 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Tue, Nov 2, 2021 at 5:57 AM Peter Geoghegan <p...@bowt.ie> wrote: > > > > > Rather than inventing PARALLEL_VACUUM_KEY_INDVAC_CHECK (just for > > assert-enabled builds), we should invent PARALLEL_VACUUM_STATS -- a > > dedicated shmem area for the array of LVSharedIndStats (no more > > storing LVSharedIndStats entries at the end of the LVShared space in > > an ad-hoc, type unsafe way). There should be one array element for > > each and every index -- even those indexes where parallel index > > vacuuming is unsafe or not worthwhile (unsure if avoiding parallel > > processing for "not worthwhile" indexes actually makes sense, BTW). We > > can then get rid of the bitmap/IndStatsIsNull() stuff entirely. We'd > > also add new per-index state fields to LVSharedIndStats itself. We > > could directly record the status of each index (e.g., parallel unsafe, > > amvacuumcleanup processing done, ambulkdelete processing done) > > explicitly. All code could safely subscript the LVSharedIndStats array > > directly, using idx style integers. That seems far more robust and > > consistent. > > Sounds good. > > During the development, I wrote the patch while considering using > fewer shared memory but it seems that it brought complexity (and > therefore the bug). It would not be harmful even if we allocate index > statistics on DSM for unsafe indexes and “not worthwhile" indexes in > practice. >
If we want to allocate index stats for all indexes in DSM then why not consider it on the lines of buf/wal_usage means tack those via LVParallelState? And probably replace bitmap with an array of bools that indicates which indexes can be skipped by the parallel worker. -- With Regards, Amit Kapila.