On Wed, Jan 17, 2024 at 4:25 PM Peter Geoghegan <p...@bowt.ie> wrote: > > On Wed, Jan 17, 2024 at 3:58 PM Robert Haas <robertmh...@gmail.com> wrote: > > > Ah, I realize I was not clear. I am now talking about inconsistencies > > > in vacuuming the FSM itself. FreeSpaceMapVacuumRange(). Not updating > > > the freespace map during the course of vacuuming the heap relation. > > > > Fair enough, but I'm still not quite sure exactly what the question > > is. It looks to me like the current code, when there are indexes, > > vacuums the FSM after each round of index vacuuming. When there are no > > indexes, doing it after each round of index vacuuming would mean never > > doing it, so instead we vacuum the FSM every ~8GB. I assume what > > happened here is that somebody decided doing it after each round of > > index vacuuming was the "right thing," and then realized that was not > > going to work if no index vacuuming was happening, and so inserted the > > 8GB threshold to cover that case. > > Note that VACUUM_FSM_EVERY_PAGES is applied against the number of > rel_pages "processed" so far -- *including* any pages that were > skipped using the visibility map. It would make a bit more sense if it > was applied against scanned_pages instead (just like > FAILSAFE_EVERY_PAGES has been since commit 07eef53955). In other > words, VACUUM_FSM_EVERY_PAGES is applied against a thing that has only > a very loose relationship with physical work performed/time elapsed.
This is a good point. Seems like a very reasonable change to make, as I would think that was the original intent. - Melanie
