Re: speed up a logical replica setup

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Euler Taveira <euler@eulerto.com>
Cc: Peter Eisentraut <peter@eisentraut.org>, pgsql-hackers@lists.postgresql.org, Amit Kapila <amit.kapila16@gmail.com>, Andres Freund <andres@anarazel.de>
Date: 2023-11-08T23:13:42Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. pg_createsubscriber: Remove obsolete comment

  2. pg_createsubscriber: Fix an unpredictable recovery wait time.

  3. Fix unstable test in 040_pg_createsubscriber.

  4. Fix the testcase introduced in commit 81d20fbf7a.

  5. Further weaken new pg_createsubscriber test on Windows.

  6. Temporarily(?) weaken new pg_createsubscriber test on Windows.

  7. Make pg_createsubscriber warn if publisher has two-phase commit enabled.

  8. Make pg_createsubscriber more wary about quoting connection parameters.

  9. pg_createsubscriber: Remove failover replication slots on subscriber

  10. pg_createsubscriber: Remove replication slot check on primary

  11. pg_createsubscriber: Only --recovery-timeout controls the end of recovery process

  12. pg_createsubscriber: creates a new logical replica from a standby server

  13. Add some const decorations

  14. Add option force_initdb to PostgreSQL::Test::Cluster:init()

  15. Remove MSVC scripts

On Wed, Nov 08, 2023 at 09:50:47AM -0300, Euler Taveira wrote:
> On Tue, Nov 7, 2023, at 8:12 PM, Michael Paquier wrote:
> I used the CreateReplicationSlot() from streamutil.h but decided to use the
> CREATE_REPLICATION_SLOT command directly because it needs the LSN as output. As
> you noticed at that time I wouldn't like a dependency in the pg_basebackup
> header files; if we move this binary to base backup directory, it seems natural
> to refactor the referred function and use it.

Right.  That should be OK to store that in an optional XLogRecPtr
pointer, aka by letting the option to pass NULL as argument of the
function if the caller needs nothing. 

>> I've read the patch, and the additions to streamutil.h and
>> streamutil.c make it kind of natural to have it sit in pg_basebackup/.
>> There's pg_recvlogical already there.  I am wondering about two
>> things, though:
>> - Should the subdirectory pg_basebackup be renamed into something more
>> generic at this point?  All these things are frontend tools that deal
>> in some way with the replication protocol to do their work.  Say
>> a replication_tools?
> 
> It is a good fit for this tool since it is another replication tool. I also
> agree with the directory renaming; it seems confusing that the directory has
> the same name as one binary but also contains other related binaries in it.

Or cluster_tools?  Or stream_tools?  replication_tools may be OK, but
I have a bad sense in naming new things around here.  So if anybody
has a better idea, feel free..

>> - And if it would be better to refactor some of the code generic to
>> all these streaming tools to fe_utils.  What makes streamutil.h a bit
>> less pluggable are all its extern variables to control the connection,
>> but perhaps that can be an advantage, as well, in some cases.
> 
> I like it. There are common functions such as GetConnection(),
> CreateReplicationSlot(), DropReplicationSlot() and RunIdentifySystem() that is
> used by all of these replication tools. We can move the extern variables into
> parameters to have a pluggable streamutil.h.

And perhaps RetrieveWalSegSize() as well as GetSlotInformation().
These kick replication commands.
--
Michael