On Wed, 9 Jun 2021 at 22:45, Peter Geoghegan <p...@bowt.ie> wrote: > > On Wed, Jun 9, 2021 at 11:45 AM Andres Freund <and...@anarazel.de> wrote: > > Good find! > > +1 > > > > The attached patch fixes this inconsistency > > > > I think I prefer applying the fix and the larger changes separately. > > I wonder if it's worth making the goto inside lazy_scan_prune verify > that the heap tuple matches what we expect. I'm sure that we would > have found this issue far sooner if that had been in place already. > Though I'm less sure of how much value adding such a check has now.
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. That is, unless we're also going to change code to update / move forward vacrel->OldestXmin in lazy_scan_prune between the HPSV call and the loop with HTSV. With regards, Matthias van de Meent
