Re: speed up a logical replica setup

On Thu, Jan 4, 2024 at 8:24 AM Euler Taveira <euler@eulerto.com> wrote:
>
> On Thu, Dec 21, 2023, at 3:16 AM, Amit Kapila wrote:
>
> > 2. remove the physical replication slot if the standby is using one
> >    (primary_slot_name).
> > 3. provide instructions to promote the logical replica into primary, I mean,
> >    stop the replication between the nodes and remove the replication setup
> >    (publications, subscriptions, replication slots). Or even include another
> >    action to do it. We could add both too.
> >
> > Point 1 should be done. Points 2 and 3 aren't essential but will provide a nice
> > UI for users that would like to use it.
> >
>
> Isn't point 2 also essential because how would otherwise such a slot
> be advanced or removed?
>
>
> I'm worried about a scenario that you will still use the primary. (Let's say
> the logical replica will be promoted to a staging or dev server.) No connection
> between primary and this new server so the primary slot is useless after the
> promotion.
>

So, you also seem to be saying that it is not required once
pg_subscriber has promoted it. So, why it should be optional to remove
physical_replication_slot? I think we must remove it from the primary
unless there is some other reason.

> A few other points:
> ==============
> 1. Previously, I asked whether we need an additional replication slot
> patch created to get consistent LSN and I see the following comment in
> the patch:
>
> + *
> + * XXX we should probably use the last created replication slot to get a
> + * consistent LSN but it should be changed after adding pg_basebackup
> + * support.
>
> Yeah, sure, we may want to do that after backup support and we can
> keep a comment for the same but I feel as the patch stands today,
> there is no good reason to keep it.
>
>
> I'll remove the comment to avoid confusing.
>

My point is to not have an additional slot and keep a comment
indicating that we need an extra slot once we add pg_basebackup
support.

>
> 2.
> + appendPQExpBuffer(str,
> +   "CREATE SUBSCRIPTION %s CONNECTION '%s' PUBLICATION %s "
> +   "WITH (create_slot = false, copy_data = false, enabled = false)",
> +   dbinfo->subname, dbinfo->pubconninfo, dbinfo->pubname);
>
> Shouldn't we enable two_phase by default for newly created
> subscriptions? Is there a reason for not doing so?
>
>
> Why? I decided to keep the default for some settings (streaming,
> synchronous_commit, two_phase, disable_on_error). Unless there is a compelling
> reason to enable it, I think we should use the default. Either way, data will
> arrive on subscriber as soon as the prepared transaction is committed.
>

I thought we could provide a better experience for logical replicas
created by default but I see your point and probably keeping default
values for parameters you mentioned seems reasonable to me.

> 3. How about sync slots on the physical standby if present? Do we want
> to retain those as it is or do we need to remove those? We are
> actively working on the patch [1] for the same.
>
>
> I didn't read the current version of the referred patch but if the proposal is
> to synchronize logical replication slots iif you are using a physical
> replication, as soon as pg_subscriber finishes the execution, there won't be
> synchronization on these logical replication slots because there isn't a
> physical replication anymore. If the goal is a promotion, the current behavior
> is correct because the logical replica will retain WAL since it was converted.
>

I don't understand what you mean by promotion in this context. If
users want to simply promote the standby then there is no need to do
additional things that this tool is doing.

> However, if you are creating a logical replica, this WAL retention is not good
> and the customer should eventually remove these logical replication slots on
> the logical replica.
>

I think asking users to manually remove such slots won't be a good
idea. We might want to either remove them by default or provide an
option to the user.

-- 
With Regards,
Amit Kapila.