Re: pg_upgrade and logical replication - Mailing list pgsql-hackers
From | Amit Kapila |
---|---|
Subject | Re: pg_upgrade and logical replication |
Date | |
Msg-id | CAA4eK1+dezyms4nydb-PiWrrgcm8reHeA8avXgKFCG+rLdnEBQ@mail.gmail.com Whole thread Raw |
In response to | Re: pg_upgrade and logical replication (Amit Kapila <amit.kapila16@gmail.com>) |
Responses |
Re: pg_upgrade and logical replication
|
List | pgsql-hackers |
On Mon, Sep 4, 2023 at 11:51 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Wed, Jul 19, 2023 at 12:47 PM Michael Paquier <michael@paquier.xyz> wrote: > > > > On Wed, May 10, 2023 at 05:59:24PM +1000, Peter Smith wrote: > > > 1. ALTER SUBSCRIPTION name ADD TABLE (relid = XYZ, state = 'x' [, lsn = 'X/Y']) > > > > > > I was a bit confused by this relation 'state' mentioned in multiple > > > places. IIUC the pg_upgrade logic is going to reject anything with a > > > non-READY (not 'r') state anyhow, so what is the point of having all > > > the extra grammar/parse_subscription_options etc to handle setting the > > > state when only possible value must be 'r'? > > > > We are just talking about the handling of an extra DefElem in an > > extensible grammar pattern, so adding the state field does not > > represent much maintenance work. I'm OK with the addition of this > > field in the data set dumped, FWIW, on the ground that it can be > > useful for debugging purposes when looking at --binary-upgrade dumps, > > and because we aim at copying catalog contents from one cluster to > > another. > > > > Anyway, I am not convinced that we have any need for a parse-able > > grammar at all, because anything that's presented on this thread is > > aimed at being used only for the internal purpose of an upgrade in a > > --binary-upgrade dump with a direct catalog copy in mind, and having a > > grammar would encourage abuses of it outside of this context. I think > > that we should aim for simpler than what's proposed by the patch, > > actually, with either a single SQL function à-la-binary_upgrade() that > > adds the contents of a relation. Or we can be crazier and just create > > INSERT queries for pg_subscription_rel to provide an exact copy of the > > catalog contents. A SQL function would be more consistent with other > > objects types that use similar tricks, see > > binary_upgrade_create_empty_extension() that does something similar > > for some pg_extension records. So, this function would require in > > input 4 arguments: > > - The subscription name or OID. > > - The relation OID. > > - Its LSN. > > - Its sync state. > > > > +1 for doing it via function (something like > binary_upgrade_create_sub_rel_state). We already have the internal > function AddSubscriptionRelState() that can do the core work. > One more related point: @@ -4814,9 +4923,31 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo) if (strcmp(subinfo->subpasswordrequired, "t") != 0) appendPQExpBuffer(query, ", password_required = false"); + if (dopt->binary_upgrade && dopt->preserve_subscriptions && + subinfo->suboriginremotelsn) + { + appendPQExpBuffer(query, ", lsn = '%s'", subinfo->suboriginremotelsn); + } Even during Create Subscription, we can use an existing function (pg_replication_origin_advance()) or a set of functions to advance the origin instead of introducing a new option. -- With Regards, Amit Kapila.
pgsql-hackers by date: