pgsql: Centralize horizon determination for temp tables, fixing bug due

From: Andres Freund <andres(at)anarazel(dot)de>
To: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: pgsql: Centralize horizon determination for temp tables, fixing bug due
Date: 2020-10-29 01:07:27
Message-ID: E1kXwPX-0005Mm-A4@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Centralize horizon determination for temp tables, fixing bug due to skew.

This fixes a bug in the edge case where, for a temp table, heap_page_prune()
can end up with a different horizon than heap_vacuum_rel(). Which can trigger
errors like "ERROR: cannot freeze committed xmax ...".

The bug was introduced due to interaction of a7212be8b9e "Set cutoff xmin more
aggressively when vacuuming a temporary table." with dc7420c2c92 "snapshot
scalability: Don't compute global horizons while building snapshots.".

The problem is caused by lazy_scan_heap() assuming that the only reason its
HeapTupleSatisfiesVacuum() call would return HEAPTUPLE_DEAD is if the tuple is
a HOT tuple, or if the tuple's inserting transaction has aborted since the
heap_page_prune() call. But after a7212be8b9e that was also possible in other
cases for temp tables, because heap_page_prune() uses a different visibility
test after dc7420c2c92.

The fix is fairly simple: Move the special case logic for temp tables from
vacuum_set_xid_limits() to the infrastructure introduced in dc7420c2c92. That
ensures that the horizon used for pruning is at least as aggressive as the one
used by lazy_scan_heap(). The concrete horizon used for temp tables is
slightly different than the logic in dc7420c2c92, but should always be as
aggressive as before (see comments).

A significant benefit to centralizing the logic procarray.c is that now the
more aggressive horizons for temp tables does not just apply to VACUUM but
also to e.g. HOT pruning and the nbtree killtuples logic.

Because isTopLevel is not needed by vacuum_set_xid_limits() anymore, I
undid the the related changes from a7212be8b9e.

This commit also adds an isolation test ensuring that the more aggressive
vacuuming and pruning of temp tables keeps working.

Debugged-By: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Debugged-By: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Debugged-By: Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>
Author: Andres Freund <andres(at)anarazel(dot)de>
Discussion: https://postgr.es/m/20201014203103.72oke6hqywcyhx7s@alap3.anarazel.de
Discussion: https://postgr.es/m/20201015083735.derdzysdtqdvxshp@alap3.anarazel.de

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/94bc27b57680b4e757577e3f5b65dc32f96d33c1

Modified Files
--------------
src/backend/access/heap/vacuumlazy.c | 1 -
src/backend/commands/cluster.c | 28 ++-
src/backend/commands/vacuum.c | 74 +++-----
src/backend/storage/ipc/procarray.c | 59 ++++++-
src/include/commands/cluster.h | 3 +-
src/include/commands/vacuum.h | 1 -
src/test/isolation/expected/horizons.out | 281 +++++++++++++++++++++++++++++++
src/test/isolation/isolation_schedule | 1 +
src/test/isolation/specs/horizons.spec | 169 +++++++++++++++++++
9 files changed, 544 insertions(+), 73 deletions(-)

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Julien Rouhaud 2020-10-29 03:34:03 Re: pgsql: Centralize horizon determination for temp tables, fixing bug due
Previous Message Michael Paquier 2020-10-29 00:29:22 pgsql: Fix incorrect placement of pfree() in pg_relation_check_pages()