Re: Emit fewer vacuum records by reaping removable tuples during pruning

On Tue, Jan 16, 2024 at 2:23 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Tue, Jan 16, 2024 at 11:28 AM Melanie Plageman
> <melanieplageman@gmail.com> wrote:
> > All LGTM.
>
> Committed.

Attached v8 patch set is rebased over this.

In 0004, I've taken the approach you seem to favor and combined the FSM
updates from the prune and no prune cases in lazy_scan_heap() instead
of pushing the FSM updates into lazy_scan_prune() and
lazy_scan_noprune().

I did not guard against calling lazy_scan_new_or_empty() a second time
in the case that lazy_scan_noprune() was called. I can do this. I
mentioned upthread I found it confusing for lazy_scan_new_or_empty()
to be guarded by if (do_prune). The overhead of calling it wouldn't be
terribly high. I can change that based on your opinion of what is
better.

The big open question/functional change is when we consider vacuuming
the FSM. Previously, we only considered vacuuming the FSM in the no
indexes, has dead items case. After combining the FSM updates from
lazy_scan_prune()'s no indexes/has lpdead items case,
lazy_scan_prune()'s regular case, and lazy_scan_noprune(), all of them
consider vacuuming the FSM. I could guard against this, but I wasn't
sure why we would want to only vacuum the FSM in the no indexes/has
dead items case.

I also noticed while rebasing something I missed while reviewing
45d395cd75ffc5b -- has_lpdead_items is set in a slightly different
place in lazy_scan_noprune() than lazy_scan_prune() (with otherwise
identical surrounding code). Both are correct, but it would have been
nice for them to be the same. If the patches below are committed, we
could standardize on the location in lazy_scan_noprune().


- Melanie