Re: pg14b1 stuck in lazy_scan_prune/heap_page_prune of pg_statistic
Matthias van de Meent <boekewurm+postgres@gmail.com>
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
To: Peter Geoghegan <pg@bowt.ie>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>,
Andres Freund <andres@anarazel.de>, Michael Paquier <michael@paquier.xyz>, Justin Pryzby <pryzby@telsasoft.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Masahiko Sawada <sawada.mshk@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2021-06-10T16:57:08Z
Lists: pgsql-hackers
On Thu, 10 Jun 2021 at 18:03, Peter Geoghegan <pg@bowt.ie> wrote: > > On Thu, Jun 10, 2021 at 8:49 AM Matthias van de Meent > <boekewurm+postgres@gmail.com> wrote: > > Could you elaborate on what this "matches what we expect" entails? > > > > Apart from this, I'm also quite certain that the goto-branch that > > created this infinite loop should have been dead code: In a correctly > > working system, the GlobalVis*Rels should always be at least as strict > > as the vacrel->OldestXmin, but at the same time only GlobalVis*Rels > > can be updated (i.e. move their horizon forward) during the vacuum. As > > such, heap_prune_satisfies_vacuum should never fail to vacuum a tuple > > that also satisifies the condition of HeapTupleSatisfiesVacuum. > > It's true that these two similar functions should be in perfect > agreement in general (given the same OldestXmin). That in itself > doesn't mean that they must always agree about a tuple in practice, > when they're called in turn inside lazy_scan_prune(). In particular, > nothing stops a transaction that was in progress to > heap_prune_satisfies_vacuum (when it saw some tuples it inserted) > concurrently aborting. That will render the same tuples fully DEAD > inside HeapTupleSatisfiesVacuum(). So we need to restart using the > goto purely to cover that case. See the commit message of commit > 8523492d4e3. I totally overlooked that HeapTupleSatisfiesVacuumHorizon does the heavyweight XID validation and does return HEAPTUPLE_DEAD in those recently rolled back cases. Thank you for reminding me. > By "matches what we expect", I meant "involves a just-aborted > transaction". We could defensively verify that the inserting > transaction concurrently aborted at the point of retrying/calling > heap_page_prune() a second time. If there is no aborted transaction > involved (as was the case with this bug), then we can be confident > that something is seriously broken. I believe there are more cases than only the rolled back case, but checking for those cases would potentially help, yes. With regards, Matthias van de Meent.
Commits
-
Deduplicate choice of horizon for a relation procarray.c.
- 3d0a4636aa4c 14.0 landed
- d9d8aa9bb9aa 15.0 landed
-
Use correct horizon when vacuuming catalog relations.
- 5a1e1d83022b 14.0 landed
-
Truncate line pointer array during VACUUM.
- 3c3b8a4b2689 14.0 cited
-
Remove tupgone special case from vacuumlazy.c.
- 8523492d4e34 14.0 cited
-
Refactor lazy_scan_heap() loop.
- 7ab96cf6b312 14.0 cited
-
snapshot scalability: Don't compute global horizons while building snapshots.
- dc7420c2c927 14.0 cited