On Fri, Jul 3, 2026 at 8:02 AM Ranier Vilela <[email protected]> wrote:
>
> Hi.
>
> lazy_scan_heap: avoid vacuum block, already vacuumed.
>
> Previously, in lazy_scan_heap() *next_fsm_block_to_vacuum*
> is set with blkno, but FreeSpaceMapVacuumRange uses an exclusive end,
> so passing blkno + 1 covers blkno itself.
>
> Advance *next_fsm_block_to_vacuum* to blkno + 1, so the next call begins
> at the first unvacuumed block and does not re-vacuum blkno.
>
> best regards,
> Ranier Vilela
>
It looks like from commit 9256822608f ("Use streaming read I/O in
VACUUM's first phase"), which converted heap_vac_scan_next_block()
into a read stream callback.
Before that commit, blkno was the block about to be processed, so
passing it as the exclusive end of the FSM range and then setting
next_fsm_block_to_vacuum = blkno was good.
Afterwards blkno became the previously processed block, and while the
call was updated to pass blkno + 1, the assignment below it was not.
So the next FreeSpaceMapVacuumRange() call needlessly re-visits the
FSM path covering blkno.
I dont think that will miss any FSM update but in the worst case,
blkno could = rel_pages - 1 and FreeSpaceMapVacuumRange will be
executed again.
The second call site a bit further down passes(Line 1562) blkno as the
(exclusive) end and then sets next_fsm_block_to_vacuum = blkno, so the
block whose free space was just recorded is not propagated upward
until the next round. That's self-consistent and loses nothing, so I
don't think it needs to change — just noting the asymmetry with the
site you're fixing, in case anyone wonders why only one of them uses +
1.
I would suggest keeping the comment simple, maybe ' Note that blkno is
the previously processed block, and that the end of the range is
exclusive.'?
Thanks,
Shihao
