On Thu, Dec 21, 2023, at 3:16 AM, Amit Kapila wrote: > I think this is an important part. Shall we try to write to some file > the pending objects to be cleaned up? We do something like that during > the upgrade.
That's a good idea. > > 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. > 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. > Also, is there a reason that we > can't create the slots after backup is complete and before we write > recovery parameters No. > 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. > 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. 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. > 4. Can we see some numbers with various sizes of databases (cluster) > to see how it impacts the time for small to large-size databases as > compared to the traditional method? This might help us with giving > users advice on when to use this tool. We can do this bit later as > well when the patch is closer to being ready for commit. I'll share it. -- Euler Taveira EDB https://www.enterprisedb.com/
