On Monday, January 29, 2024 7:30 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Jan 29, 2024 at 3:11 PM shveta malik <shveta.ma...@gmail.com> > wrote: > > > > PFA v71 patch set with above changes. > > > > Few comments on 0001
Thanks for the comments. > =================== > 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. > > 2. > @@ -148,6 +153,10 @@ typedef struct Subscription > List *publications; /* List of publication names to subscribe to */ > char *origin; /* Only publish data originating from the > * specified origin */ > + bool failover; /* True if the associated replication slots > + * (i.e. the main slot and the table sync > + * slots) in the upstream database are enabled > + * to be synchronized to the standbys. */ > } Subscription; > > Let's add this new field immediately after "bool runasowner;" as is done for > other boolean members. This will help avoid increasing the size of the > structure > due to alignment when we add any new pointer field in the future. Also, that > would be consistent with what we do for other new boolean members. Moved this field as suggested. Attach the V72-0001 which addressed above comments, other patches will be rebased and posted after pushing first patch. Thanks Shveta for helping address the comments. Best Regards, Hou zj
v72-0001-Add-a-failover-option-to-subscriptions.patch
Description: v72-0001-Add-a-failover-option-to-subscriptions.patch