Re: Periodic FSM vacuum doesn't happen in one-pass strategy vacuum.

On Mon, Jun 9, 2025 at 6:22 PM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
>
> Reviewing your patch, I think there might be an issue still. You
> replaced has_lpdead_items with ndeleted. While ndeleted will count
> those items we set LP_UNUSED (which is what we want), it also counts
> LP_NORMAL items that vacuum sets LP_REDIRECT (see
> heap_prune_record_redirect()).
>
> Looking at PageGetHeapFreeSpace(), it seems like it only considers
> LP_UNUSED items as freespace. So, if an item is set LP_REDIRECT, there
> would be no need to update the FSM, I think.

I was wrong. Setting an item to LP_REDIRECT does free up space --
because there is no associated storage. PageGetFreeSpace() is
concerned with pd_upper and pd_lower. So after setting an item
LP_REDIRECT, we'll compactify_tuples() in PageRepairFragmentation(),
altering pd_upper/lower. Then future calls to PageGetHeapFreeSpace()
will report this updated number and we'll put that in the freespace
map. So, we can expect setting items LP_REDIRECT will result in new
freespace to report.

(I got confused by some code in PageGetHeapFreeSpace() that was
checking to make sure that, if there were >= MaxHeapTuplesPerPage line
pointers, that at least one of them is LP_UNUSED).

So, I think we should commit the fix you proposed.

The only question I have left is implementation: should we have
ndeleted as an output parameter of lazy_scan_prune() or have
lazy_scan_prune() return it (instead of void)?

In <= 16, heap_page_prune() returned the number of tuples deleted, so
I thought of maybe having lazy_scan_prune() do this. Though, maybe it
is confusing to have one result returned as the return value and the
others returned in output parameters unless there is something more
special about ndeleted. With heap_page_prune(), I think it was the
return value because that was kind of what heap_page_prune()
"accomplished".

- Melanie