Re: Allow aggressive VACUUM to freeze without a cleanup lock

Hi Melanie, Peter, Jingtang,

> So it's reasonable to surmise that freezing could be done with only an
> exclusive lock on top of the current VACUUM code (though not a shared
> lock, which seems really dangerous). And that page defragmentation is
> the only thing that necessitates a cleanup lock/makes this idea difficult.

Agreed. Freezing itself only needs an exclusive lock, a cleanup lock is needed
for page defragmentation. I also agree with Melanie that the patch should
explain this if it is not already documented elsewhere.

I reviewed the patch and found a few issues.

First, the buffer lock is released and reacquired between
`lazy_scan_noprune()` and `lazy_scan_freeze()`. The page may be changed or
already frozen during this window, so this assertion is not always valid:
```C
Assert(state.pagefrz.freeze_required);
```

Second, the code may call heap_prepare_freeze_tuple() and then discard the
freeze plans after finding an old DEAD tuple. This function may create a new
MultiXactId, write WAL, and advance the MultiXact state. These side effects
remain even if the plans are discarded.

There is also a small naming issue. lazy_scan_noprune and lazy_scan_freeze are
quite confusing now. I think lazy_scan_noprune is not correct at this moment.

The patch adds FreezeState, HeapPageFreezeParams, and
HeapPageFreezeResult, but they overlap with PruneState,
PruneFreezeParams, and PruneFreezeResult. These abstractions add complexity
without much reuse.

Pages containing old XIDs or MultiXactIds in DEAD tuples still need a cleanup
lock. I think this is a reasonable limitation for this patch.

Best Regards,
Yuhang Qiu