Re: relfrozenxid may disagree with row XIDs after 1ccc1e05ae

Melanie Plageman <melanieplageman@gmail.com>

From: Melanie Plageman <melanieplageman@gmail.com>

To: Noah Misch <noah@leadboat.com>

Cc: Bowen Shi <zxwsbg12138@gmail.com>, Andres Freund <andres@anarazel.de>, Robert Haas <robertmhaas@gmail.com>, Matthias van de Meent <boekewurm+postgres@gmail.com>, Peter Geoghegan <pg@bowt.ie>, Alexander Lakhin <exclusion@gmail.com>, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>

Date: 2024-07-08T21:23:52Z

Lists: pgsql-bugs

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Ensure vacuum removes all visibly dead tuples older than OldestXmin
- 06bf404cd07b 16.4 landed
- 45ce054c02b8 14.13 landed
- dc6354c67017 15.8 landed
Combine freezing and pruning steps in VACUUM
- 6dbb490261a6 17.0 cited
Handle non-chain tuples outside of heap_prune_chain()
- 6f47f6883151 17.0 cited
Fix false reports in pg_visibility
- e85662df44ff 17.0 cited
Remove retry loop in heap_page_prune().
- 1ccc1e05ae8f 17.0 cited
vacuumlazy.c: document vistest and OldestXmin.
- 73f6ec3d3c8d 15.0 cited
Deduplicate choice of horizon for a relation procarray.c.
- d9d8aa9bb9aa 15.0 cited
Remove tupgone special case from vacuumlazy.c.
- 8523492d4e34 14.0 cited
Simplify state managed by VACUUM.
- b4af70cb2103 14.0 cited
Recycle nbtree pages deleted during same VACUUM.
- 9dd963ae2534 14.0 cited
snapshot scalability: Don't compute global horizons while building snapshots.
- dc7420c2c927 14.0 cited
Raise error when affecting tuple moved into different partition.
- f16241bef7cc 11.0 cited

Attachments

REL14_v2-0001-Test-that-vacuum-removes-tuples-older-than-Oldest.patch (text/x-patch) patch v2-0001
REL15_v2-0002-Ensure-vacuum-removes-all-visibly-dead-tuples-old.patch (text/x-patch) patch v2-0002
REL14_v2-0002-Ensure-vacuum-removes-all-visibly-dead-tuples-old.patch (text/x-patch) patch v2-0002
REL15_v2-0001-Test-that-vacuum-removes-tuples-older-than-Oldest.patch (text/x-patch) patch v2-0001
REL16_v2-0001-Test-that-vacuum-removes-tuples-older-than-Oldest.patch (text/x-patch) patch v2-0001
REL16_v2-0002-Ensure-vacuum-removes-all-visibly-dead-tuples-old.patch (text/x-patch) patch v2-0002
REL17_v2-0001-Test-that-vacuum-removes-tuples-older-than-Oldest.patch (text/x-patch) patch v2-0001
REL17_v2-0002-Ensure-vacuum-removes-all-visibly-dead-tuples-old.patch (text/x-patch) patch v2-0002

On Tue, Jun 25, 2024 at 3:37 PM Noah Misch <noah@leadboat.com> wrote:
>
> On Thu, Jun 20, 2024 at 11:49:50AM -0400, Melanie Plageman wrote:
> > On Tue, Jun 18, 2024 at 6:51 PM Melanie Plageman <melanieplageman@gmail.com> wrote:
> > > I ended up manually backporting the logic from 1ccc1e05ae as opposed
> > > to cherry-picking because it relied on a struct introduced in
> > > 4e9fc3a9762065.
>
> > Attached is the backport and repros for 15 and 16.

I think we are going with the fix proposed for master [1] which
compares dead_after to OldestXmin before using GlobalVisState.
Backporting 1ccc1e05ae doesn't actually fix the problem. We just end
up erroring out when attempting to freeze the tuple we didn't remove.

As such, attached is my proposed fix for affected stable branches. It
is based off of the fix proposed in [1] but is a bit different in each
version due to surrounding code changes.

The test I added passes locally and on linux and windows in CI (on 15+
which have CI). I don't have enough cirrus credits to run the tests on
mac. I am nervous about the test flaking on the buildfarm. But I did
the best I could to try to make it stable. I think keeping it as a
separate commit should be easiest in case we have to revert it?

Thanks to Heikki for backporting BackgroundPsql -- this made my life
much easier!!

- Melanie

[1] https://www.postgresql.org/message-id/CAAKRu_Z4PybtZ0i_NKOr-vbrFW5p1ZdfEfUqaeU8fLPhszpP_g%40mail.gmail.com