public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: David Christensen <[email protected]>
Cc: Bharath Rupireddy <[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: Mon, 19 Dec 2022 15:22:59 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAOxo6X+NCVAB3H5_TMSasgimf127KURoi=S0dTSNJUn8Z8+WNA@mail.gmail.com>
References: <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 Thu, Dec 15, 2022 at 05:17:46PM -0600, David Christensen wrote:
> On Thu, Dec 15, 2022 at 12:36 AM Michael Paquier <[email protected]> wrote:
> This v10 should incorporate your feedback as well as Bharath's.
Thanks for the new version. I have minor comments.
>> It seems to me that you could allow things to work even if the
>> directory exists and is empty. See for example
>> verify_dir_is_empty_or_create() in pg_basebackup.c.
>
> The `pg_mkdir_p()` supports an existing directory (and I don't think
> we want to require it to be empty first), so this only errors when it
> can't create a directory for some reason.
Sure, but things can also be made so as we don't fail if the directory
exists and is empty? This would be more consistent with the base
directories created by pg_basebackup and initdb.
>> +$node->safe_psql('postgres', <<EOF);
>> +SELECT 'init' FROM pg_create_physical_replication_slot('regress_pg_waldump_slot', true, false);
>> +CREATE TABLE test_table AS SELECT generate_series(1,100) a;
>> +CHECKPOINT; -- required to force FPI for next writes
>> +UPDATE test_table SET a = a + 1;
>> Using an EOF to execute a multi-line query would be a first. Couldn't
>> you use the same thing as anywhere else? 009_twophase.pl just to
>> mention one. (Mentioned by Bharath upthread, where he asked for an
>> extra opinion so here it is.)
>
> Fair enough, while idiomatic perl to me, not a hill to die on;
> converted to a standard multiline string.
By the way, knowing that we have an option called --fullpage, could be
be better to use --save-fullpage for the option name?
+ OPF = fopen(filename, PG_BINARY_W);
+ if (!OPF)
+ pg_fatal("couldn't open file for output: %s", filename);
[..]
+ if (fwrite(page, BLCKSZ, 1, OPF) != 1)
+ pg_fatal("couldn't write out complete full page image to file: %s", filename);
These should more more generic, as of "could not open file \"%s\"" and
"could not write file \"%s\"" as the file name provides all the
information about what this writes.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
download
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: <[email protected]>
* 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