Hi, On 2026-02-27 18:56:48 -0500, Melanie Plageman wrote: > On Tue, Dec 2, 2025 at 2:39 PM Heikki Linnakangas <[email protected]> wrote: > > > > A prune record can have XLogRecord->xl_xid == InvalidTransactionId, if > > the transaction hasn't been assigned a transaction ID yet. I think > > ReadNextTransactionId() - 1 would work. (Using TransactionIdRetreat > > rather than plain - 1, of course) > > I was thinking about this some more, and I propose we just clear > pd_prune_xid whenever we set PD_ALL_VISIBLE like in the attached > patch.
Think that makes sense. > I take your point that it could be worth setting pd_prune_xid to > something on the standby when there are prunable tuples left so that > we trigger a prune cycle after promotion, but since Andres didn't like > that idea and we'll update pd_prune_xid on the next delete/update > anyway, I think we don't need to do it. I mean I mainly disliked it because I think it'd be better if we "just" ensure the values are the same (with perhaps an exception for it being unset on the primary and set on the standby, if the pd_prune_xid is cleared without WAL logging). If we don't do that, I think setting it to something vaguely useful is better than not doing anything. > Regarding Andres' idea to include pd_prune_xid to the xl_heap_prune > WAL record: I don't feel quite comfortable that pd_prune_xid would > then be half WAL-logged (since it isn't logged when update/delete sets > it). Maybe it's fine, though. What do you mean with half WAL logged? Just that PageSetPrunable() will take the current pd_prune_xid into account, which could differ on the standby? We'd still not put in an XID that wasn't in a WAL record, right? I think the case of xl_heap_prune is markedly different from e.g. the delete case, because the choice of the xid to use for pd_prune_xid is a lot more complicated in the other cases. Without inspecting all the surviving rows, the standby just don't have a sane value to put into pd_prune_xid. Whereas for a delete, update, insert we know that it's just the smaller of the current pd_prune_xid and the xid of the record. > diff --git a/src/backend/access/heap/vacuumlazy.c > b/src/backend/access/heap/vacuumlazy.c > index dcbaf49401b..88bcbf55ed7 100644 > --- a/src/backend/access/heap/vacuumlazy.c > +++ b/src/backend/access/heap/vacuumlazy.c > @@ -1932,6 +1932,7 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, > BlockNumber blkno, > log_newpage_buffer(buf, true); > > PageSetAllVisible(page); > + PageClearPrunable(page); > visibilitymap_set(vacrel->rel, blkno, buf, > InvalidXLogRecPtr, > vmbuffer, > InvalidTransactionId, > @@ -2943,6 +2944,7 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber > blkno, Buffer buffer, > * set PD_ALL_VISIBLE. > */ > PageSetAllVisible(page); > + PageClearPrunable(page); > visibilitymap_set_vmbits(blkno, > vmbuffer, > vmflags, > > vacrel->rel->rd_locator); What about the PageSetAllVisible() in lazy_scan_prune()? I know that code will vanish soon, but still... Greetings, Andres Freund
