On Fri, Apr 4, 2025 at 6:07 PM Masahiko Sawada <sawada.mshk@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