On 2021/07/13 5:57, Matthias van de Meent wrote: > > > On Mon, 12 Jul 2021 at 13:14, <ikeda...@oss.nttdata.com > <mailto:ikeda...@oss.nttdata.com>> wrote: >> >> Hi, >> >> While I’m reading source codes related to vacuum, I found comments which >> don’t seem to fit the reality. I think the commit[1] just forgot to fix them. >> What do you think? > > Hmm, yes, those are indeed some leftovers. > > Some comments on the suggested changes: > > > - * caused by HeapTupleSatisfiesVacuum. We just add entries to the arrays in > + * caused by heap_prune_satisfies_vacuum. We just add entries to the arrays > in > > I think that HeapTupleSatisfiesVacuumHorizon might be an alternative correct > replacement here. > > > - elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result"); > + elog(ERROR, "unexpected heap_prune_satisfies_vacuum result"); > > The type of the value is HTSV_Result; where HTSV stands for > HeapTupleSatisfiesVacuum, so if we were to replace this, I'd go for > "unexpected result from heap_prune_satisfies_vacuum" as a message instead.
Thanks for your comments. I agree your suggestions. I also updated prstate->vistest to heap_prune_satisfies_vacuum of v1 patch because heap_prune_satisfies_vacuum() tests with not only prstate->vistest but also prstate->old_snap_xmin. I think it's more accurate representation. Regards, -- Masahiro Ikeda NTT DATA CORPORATION
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index 15ca1b304a..d2f4c30baa 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -488,17 +488,15 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer) * the HOT chain is pruned by removing all DEAD tuples at the start of the HOT * chain. We also prune any RECENTLY_DEAD tuples preceding a DEAD tuple. * This is OK because a RECENTLY_DEAD tuple preceding a DEAD tuple is really - * DEAD, the OldestXmin test is just too coarse to detect it. + * DEAD, heap_prune_satisfies_vacuum() is just too coarse to detect it. * * The root line pointer is redirected to the tuple immediately after the * latest DEAD tuple. If all tuples in the chain are DEAD, the root line * pointer is marked LP_DEAD. (This includes the case of a DEAD simple * tuple, which we treat as a chain of length 1.) * - * OldestXmin is the cutoff XID used to identify dead tuples. - * * We don't actually change the page here, except perhaps for hint-bit updates - * caused by HeapTupleSatisfiesVacuum. We just add entries to the arrays in + * caused by HeapTupleSatisfiesVacuumHorizon. We just add entries to the arrays in * prstate showing the changes to be made. Items to be redirected are added * to the redirected[] array (two entries per redirection); items to be set to * LP_DEAD state are added to nowdead[]; and items to be set to LP_UNUSED @@ -680,7 +678,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum, PruneState *prstate) break; default: - elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result"); + elog(ERROR, "unexpected result from heap_prune_satisfies_vacuum"); break; }
