On Mon, Apr 7, 2025 at 11:52 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> 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.
I think that this issue presents since commit c120550edb86 but this
commit optimized the vacuum work for tables with no indexes and wasn't
intended to change the FSM vacuum behavior for such tables. Therefore,
I think we can fix the FSM vacuum to restore the original FSM vacuum
behavior for HEAD and v18, leaving aside the fact that we might want
to fix/improve VACUUM_FSM_EVERY_PAGES optimization . The original FSM
vacuum strategy for tables with no index was that we call
FreeSpaceMapVacuumRange() for every VACUUM_FSM_EVERY_PAGES, followed
by a final FreeSpaceMapVacuumRange() call for any remaining pages at
end of lazy_scan_heap():
/*
* Vacuum the remainder of the Free Space Map. We must do this whether or
* not there were indexes, and whether or not we bypassed index vacuuming.
* We can pass rel_pages here because we never skip scanning the last
* block of the relation.
*/
if (rel_pages > next_fsm_block_to_vacuum)
FreeSpaceMapVacuumRange(vacrel->rel, next_fsm_block_to_vacuum,
rel_pages);
Therefore, we do FSM vacuum for the entire table after all even if the
table has no garbage. As for VACUUM_FSM_EVERY_PAGES optimization, we
call FreeSpaceMapVacuumRange() only if we remove at least one tuple
from the page. I think these two points should have been maintained
post-commit. I've attached the patch implementing this idea.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
