On Mon, Apr 7, 2025 at 8:30 AM Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> 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).
Thank you for referring to the thread. I'll read through the discussion.
> 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.
You're right. It would be better to replace it with something as you
suggested instead of just removing it. On the other hand, IIUC even if
there is no garbage on the table, we do FSM vacuum anyway at the end
of lazy_scan_heap(). Doing FSM vacuum periodically regardless of
presence of garbage might help load the disk I/O for FSM vacuum. But
we might think it is rather a bug that we do FSM vacuum even on an
all-frozen table.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com