On Fri, Apr 4, 2025 at 6:07 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > I'm going to push this fix up to HEAD and v17 early next week, unless > there is no objection.
I started studying this again looking back at the thread associated with commit c120550edb86. There was actually a long discussion about how this commit interacted with how often the freespace map was vacuumed. [1] is one of the emails addressing that issue. If you grep for FreeSpaceMapVacuumRange() on the thread, you can see the replies to this topic, as they are interspersed with replies about updating the FSM (as opposed to vacuuming it). What I'm wondering is won't the patch you propose: simply removing has_lpdead_items from if (got_cleanup_lock && vacrel->nindexes == 0 && has_lpdead_items && blkno - next_fsm_block_to_vacuum >= VACUUM_FSM_EVERY_PAGES) lead to vacuuming the FSM when there is nothing to vacuum and thus to wasting IO (when we didn't set anything to LP_UNUSED). It seems like the logic we would want is to replace has_lpdead_items with something about having set items lpunused. Rereading that thread, it seems we discussed what the right logic for this was extensively, but I don't quite understand how we ended up with if (got_cleanup_lock && vacrel->nindexes == 0 && has_lpdead_items && - Melanie [1] https://www.postgresql.org/message-id/CA%2BTgmoYV5LFUgXS_g4S9P1AhQ63thPg6UJj8EsmsmEahFxLY_A%40mail.gmail.com