On Mon, Jan 29, 2024 at 6:47 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> On Monday, January 29, 2024 7:30 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
>
> > ===================
> > 1.
> > parse_subscription_options()
> > {
> > ...
> > /*
> > * We've been explicitly asked to not connect, that requires some
> > * additional processing.
> > */
> > if (!opts->connect && IsSet(supported_opts, SUBOPT_CONNECT)) {
> >
> > Here, along with other options, we need an explicit check for failover, so that if
> > connect=false and failover=true, the statement should give error. I was
> > expecting the below statement to fail but it passed with WARNING.
> > postgres=# create subscription sub2 connection 'dbname=postgres'
> > publication pub2 with(connect=false, failover=true);
> > WARNING: subscription was created, but is not connected
> > HINT: To initiate replication, you must manually create the replication slot,
> > enable the subscription, and refresh the subscription.
> > CREATE SUBSCRIPTION
>
> Added.
>
In this regard, I feel we don't need to dump/restore the 'FAILOVER'
option non-binary upgrade paths similar to the 'ENABLE' option. For
binary upgrade, if the failover option is enabled, then we can enable
it using Alter Subscription SET (failover=true). Let's add one test
corresponding to this behavior in
postgresql\src\bin\pg_upgrade\t\004_subscription.
Additionally, we need to update the pg_dump docs for the 'failover'
option. See "When dumping logical replication subscriptions, .." [1].
I think we also need to update the connect option docs in CREATE
SUBSCRIPTION [2].
[1] - https://www.postgresql.org/docs/devel/app-pgdump.html
[2] - https://www.postgresql.org/docs/devel/sql-createsubscription.html
--
With Regards,
Amit Kapila.
