Re: pg_receivewal starting position

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