On Wed, 9 Jun 2021 at 22:45, Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Wed, Jun 9, 2021 at 11:45 AM Andres Freund <andres@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