From: Ronan Dunklau <ronan.dunklau@aiven.io>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@lists.postgresql.org, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Subject: Re: pg_receivewal starting position
Date: Mon, 25 Oct 2021 09:15:32 +0200
Message-ID: <5781690.lOV4Wx5bFT@aivenronan> (raw)
In-Reply-To: <YXZT64bdcSBQIJpj@paquier.xyz>
References: <CALj2ACW6YA0SfqwPm1-FRRdeNj1e9isiSax3jZ8MmyQYYQQWNw@mail.gmail.com>
<4126495.44csPzL39Z@aivenronan>
<YXZT64bdcSBQIJpj@paquier.xyz>
Le lundi 25 octobre 2021, 08:51:23 CEST Michael Paquier a écrit :
> On Thu, Oct 21, 2021 at 10:36:42AM +0200, Ronan Dunklau wrote:
> > Done. I haven't touched the timeline switch test patch for now, but I
> > still
> > include it here for completeness.
>
> As 0001 and 0002 have been applied, I have put my hands on 0003, and
> found a couple of issues upon review.
>
> + Assert(slot_name != NULL);
> + Assert(restart_lsn != NULL);
> There is no need for those asserts, as we should support the case
> where the caller gives NULL for those variables.
Does it make sense though ? The NULL slot_name case handling is pretty
straight forward has it will be handled by string formatting, but in the case
of a null restart_lsn, we have no way of knowing if the command was issued at
all.
>
> + if (PQserverVersion(conn) < 150000)
> + return false;
> Returning false is incorrect for older server versions as we won't
> fallback to the old method when streaming from older server. What
> this needs to do is return true and set restart_lsn to
> InvalidXLogRecPtr, so as pg_receivewal would just stream from the
> current flush location. "false" should just be returned on error,
> with pg_log_error().
Thank you, this was an oversight when moving from the more complicated error
handling code.
>
> +$primary->psql('postgres',
> + 'INSERT INTO test_table VALUES (generate_series(1,100));');
> +$primary->psql('postgres', 'SELECT pg_switch_wal();');
> +$nextlsn =
> + $primary->safe_psql('postgres', 'SELECT pg_current_wal_insert_lsn();');
> +chomp($nextlsn);
> There is no need to switch twice to a new WAL segment as we just need
> to be sure that the WAL segment of the restart_lsn is the one
> archived. Note that RESERVE_WAL uses the last redo point, so it is
> better to use a checkpoint and reduce the number of logs we stream
> into the new location.
>
> Better to add some --no-sync to the new commands of pg_receivewal, to
> not stress the I/O more than necessary. I have added some extra -n
> while on it to avoid loops on failure.
>
> Attached is the updated patch I am finishing with, which is rather
> clean now. I have tweaked a couple of things while on it, and
> documented better the new GetSlotInformation() in streamutil.c.
> --
> Michael
--
Ronan Dunklau
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: pgsql-hackers@postgresql.org
Cc: ronan.dunklau@aiven.io, michael@paquier.xyz, bharath.rupireddyforpostgres@gmail.com, horikyota.ntt@gmail.com, sawada.mshk@gmail.com
Subject: Re: pg_receivewal starting position
In-Reply-To: <5781690.lOV4Wx5bFT@aivenronan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox