(Thanks for the reminder on this, Michael!)

On 05/04/2019 08:39, Andrey Borodin wrote:
4 апр. 2019 г., в 20:15, Heikki Linnakangas <hlinn...@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


Reply via email to