Re: Cleaning up nbtree after logical decoding on standby work

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Peter Geoghegan <pg(at)bowt(dot)ie>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Drouvot, Bertrand" <bertranddrouvot(dot)pg(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: Cleaning up nbtree after logical decoding on standby work
Date: 2023-05-26 07:46:31
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: 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.

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2023-05-26 08:09:35 Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?
Previous Message Peter Smith 2023-05-26 07:29:38 Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication