Re: Cleaning up nbtree after logical decoding on standby work - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Cleaning up nbtree after logical decoding on standby work
Date
Msg-id ZHBj13xvOa8AgdRE@paquier.xyz
Whole thread Raw
In response to Cleaning up nbtree after logical decoding on standby work  (Peter Geoghegan <pg@bowt.ie>)
Responses Re: Cleaning up nbtree after logical decoding on standby work
List pgsql-hackers
On Thu, May 25, 2023 at 06:50:31PM -0700, Peter Geoghegan wrote:
> It's possible that Bertand would have done it this way to begin with
> were it not for the admittedly pretty bad nbtree convention around
> P_NEW. It would be nice to get rid of P_NEW in the near future, too --
> I gather that there was discussion of that in the context of recent
> work in this area.

Nice cleanup overall.

+    if (XLogStandbyInfoActive() && RelationNeedsWAL(rel))
     {
-        /* Okay to use page.  Initialize and return it. */
-        _bt_pageinit(page, BufferGetPageSize(buf));
-        return buf;
+        safexid = BTPageGetDeleteXid(page);
+        isCatalogRel = RelationIsAccessibleInLogicalDecoding(heaprel);
+        _bt_log_reuse_page(rel, blkno, safexid, isCatalogRel);

There is only one caller of _bt_log_reuse_page(), so assigning a
boolean rather than the heap relation is a bit strange to me.  I think
that calling RelationIsAccessibleInLogicalDecoding() within
_bt_log_reuse_page() where xlrec_reuse is filled with its data is much
more natural, like HEAD.  One argument in favor of HEAD is that it is
not possible to pass down a wrong value for isCatalogRel, but your
patch would make that possible.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Peter Smith
Date:
Subject: Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication
Next
From: Daniel Gustafsson
Date:
Subject: Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?