Re: collect_corrupt_items_vacuum.patch

Hi!

On Tue, Dec 5, 2023 at 9:03 PM Dmitry Koval <d(dot)koval(at)postgrespro(dot)ru> wrote:
> I agree with Alexander Lakhin about PROC_IN_VACUUM and
> VISHORIZON_DATA_STRICT:
> 1) probably manipulations with the PROC_IN_VACUUM flag in
> pg_visibility.c were needed for condition [1] and can be removed now;

Right, PROC_IN_VACUUM is no longer required. The possible benefit of
it would be to avoid bloat during a possibly long run of
pg_visibility() function. But the downside are problems with the
snapshot if the invoking query contains something except a single call
of the pg_visibility() function, and complexity. Removed.

> 2) the VISHORIZON_DATA_STRICT macro is probably unnecessary too (since
> we are not going to use it in the GlobalVisHorizonKindForRel() function).

Makes sense, removed.

> Also It would be nice to remove the get_strict_xid_horizon() function
> from the comment (replace to GetStrictOldestNonRemovableTransactionId()?).

Right, fixed.

The revised patch is attached. Besides the fixes above, it contains
improvements for comments and the detailed commit message.

Tom, Robert, what do you think about the patch attached? It required
a new type of xid horizon in core and sacrifices accuracy. But this
is the only way I can imagine, we can fix the problem in a general
way.

------
Regards,
Alexander Korotkov