public inbox for [email protected]  
help / color / mirror / Atom feed
From: Bharath Rupireddy <[email protected]>
To: David Christensen <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Justin Pryzby <[email protected]>
Cc: Ian Lawrence Barwick <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] Teach pg_waldump to extract FPIs from the WAL
Date: Wed, 21 Dec 2022 17:17:01 +0530
Message-ID: <CALj2ACUT+DB8PhF6ja8f-LP76cEappXrJuhzrKd81a90UmrnYA@mail.gmail.com> (raw)
In-Reply-To: <CAOxo6X+NCVAB3H5_TMSasgimf127KURoi=S0dTSNJUn8Z8+WNA@mail.gmail.com>
References: <CALj2ACWTQCYGHf=2pHQeX=8qfkMi-4JsXN0t_6W8+A-cW8wJpw@mail.gmail.com>
	<CAOxo6X+wXEu1QfTFTf-WiDs3jr8ERvgEOddHfUhAH_5eje-G2w@mail.gmail.com>
	<CAOxo6XJ03w=PWDnsnzDpGK+4Er7XZwFkhqiMbVWmt7VLHzidHw@mail.gmail.com>
	<CALj2ACUukW74XXb2n3FFAdRiisF06jSNDR8DegOpMevq_0yVoA@mail.gmail.com>
	<CAOxo6XJ+uN-kiCS9wVKm2LudZXEZOk6zzduF-uGc_4464uXs9w@mail.gmail.com>
	<CAOxo6X+ALOqqZ+a1r-=BTDaUB6LL0w7nnu=K_kvYbGsrkU2eyA@mail.gmail.com>
	<CALj2ACV4donjBLaxkJ1FO3Ybg73ut--9TozHTsNyBVCdX9opYA@mail.gmail.com>
	<CAOxo6XLhduEAn0yLh+BGOjpq6k0x9MsOY6PjDEoCh4VvdUxCiQ@mail.gmail.com>
	<CALj2ACWmgbDeCDMaZah0GRwQBCJMD01U4S69V6TE4W2jYmnLuQ@mail.gmail.com>
	<CAOxo6XJMcLyLb1D4oOV2FHfJvt1Z6DuediGUKKEHAEMvpPe2ow@mail.gmail.com>
	<[email protected]>
	<CAOxo6X+NCVAB3H5_TMSasgimf127KURoi=S0dTSNJUn8Z8+WNA@mail.gmail.com>

On Fri, Dec 16, 2022 at 4:47 AM David Christensen
<[email protected]> wrote:
>
> On Thu, Dec 15, 2022 at 12:36 AM Michael Paquier <[email protected]> wrote:
> >
> > On Wed, Dec 14, 2022 at 04:44:34PM -0600, David Christensen wrote:
> > > I can get one sent in tomorrow.
>
> This v10 should incorporate your feedback as well as Bharath's.

Thanks for the patch. Here're some minor comments:

1. +my $node =  PostgreSQL::Test::Cluster->new('primary');
Can the name be other than 'primary' because we don't create a standby
for this test? Something like - 'node_a' or 'node_extract_fpi' or some
other.

2. +$node->init(extra => ['-k'], allows_streaming => 1);
When enabled with allows_streaming, there are a bunch of things that
happen to the node while initializing, I don't think we need all of
them for this.

3. +$node->init(extra => ['-k'], allows_streaming => 1);
Can we use --data-checksums instead of -k for more readability?
Perhaps, a comment on why we need that option helps greatly.

4.
+        page = (Page) buf.data;
+
+        if (!XLogRecHasBlockRef(record, block_id))
+            continue;
+
+        if (!XLogRecHasBlockImage(record, block_id))
+            continue;
+
+        if (!RestoreBlockImage(record, block_id, page))
+            continue;
Can you shift  page = (Page) buf.data; just before the last if
condition RestoreBlockImage() so that it doesn't get executed for the
other two continue statements?

-- 
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com





view thread (55+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Teach pg_waldump to extract FPIs from the WAL
  In-Reply-To: <CALj2ACUT+DB8PhF6ja8f-LP76cEappXrJuhzrKd81a90UmrnYA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox