(Thanks for the reminder on this, Michael!)
On 05/04/2019 08:39, Andrey Borodin wrote:
>> 4 апр. 2019 г., в 20:15, Heikki Linnakangas <hlinnaka@iki.fi> написал(а):
>> I suggest that we do the attached. It fixes this for GiST. The
>> patch changes expands the "deletion XID" to 64-bits, and changes
>> where it's stored. Instead of storing it pd_prune_xid, it's stored
>> in the page contents. Luckily, a deleted page has no real content.
>
> So, we store full xid right after page header?
Yep.
> +static inline void
> +GistPageSetDeleteXid(Page page, FullTransactionId deletexid)
> +{
> + Assert(PageIsEmpty(page));
> + ((PageHeader) page)->pd_lower = MAXALIGN(SizeOfPageHeaderData) + sizeof(FullTransactionId);
> +
> + *((FullTransactionId *) PageGetContents(page)) = deletexid;
> +}
>
> Usually we leave one ItemId (located at invalid offset number)
> untouched. I do not know is it done for a reason or not....
No. Take a look at PageGetItemId() macro, it subtracts one from the
offset number. But in any case, that's not really relevant here, because
this patch stores the transaction id directly as the page content. There
are no itemids at all on a deleted page.
> Also, I did not understand this optimization:
> + /*
> + * We can skip this if the page was deleted so long ago, that no scan can possibly
> + * still see it, even in a standby. One measure might be anything older than the
> + * table's frozen-xid, but we don't have that at hand here. But anything older than
> + * 2 billion, from the next XID, is surely old enough, because you would hit XID
> + * wraparound at that point.
> + */
> + nextxid = ReadNextFullTransactionId();
> + diff = U64FromFullTransactionId(nextxid) - U64FromFullTransactionId(latestRemovedXid);
> + if (diff < 0x7fffffff)
> + return;
>
> Standby can be lagging months from primary, and, theoretically, close
> the gap in one sudden WAL leap...
It would still process the WAL one WAL record at a time, even if it's
lagging months behind. It can't just jump over 2 billion XIDs.
> Also, I think, that comparison sign should be >, not <.
Ah, good catch! And it shows that this needs more testing..
- Heikki
