From: | vignesh C <vignesh21(at)gmail(dot)com> |
---|---|
To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, Jaime Casanova <jcasanov(at)systemguards(dot)com(dot)ec>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Japin Li <japinli(at)hotmail(dot)com>, Euler Taveira <euler(at)eulerto(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Identify missing publications from publisher while create/alter subscription. |
Date: | 2022-03-30 16:24:00 |
Message-ID: | CALDaNm00WeabS5ULrXA-1UD8Bt=4w9inQRWyydUSpHGO5a5yJw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Mar 30, 2022 at 5:42 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> On Wed, Mar 30, 2022 at 5:37 PM Bharath Rupireddy
> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> >
> > On Wed, Mar 30, 2022 at 4:29 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Wed, Mar 30, 2022 at 12:22 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > > >
> > > > I have made the changes for this, attached v17 patch has the changes
> > > > for the same.
> > > >
> > >
> > > The patch looks good to me. I have made minor edits in the comments
> > > and docs. See the attached and let me know what you think? I intend to
> > > commit this tomorrow unless there are more comments or suggestions.
> >
> > I have one minor comment:
> >
> > + "Create subscription throws warning for multiple non-existent publications");
> > +$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION mysub1;");
> > + "CREATE SUBSCRIPTION mysub1 CONNECTION '$publisher_connstr'
> > PUBLICATION mypub;"
> > + "ALTER SUBSCRIPTION mysub1 ADD PUBLICATION non_existent_pub"
> > + "ALTER SUBSCRIPTION mysub1 SET PUBLICATION non_existent_pub"
> >
> > Why should we drop the subscription mysub1 and create it for ALTER ..
> > ADD and ALTER .. SET tests? Can't we just do below which saves
> > unnecessary subscription creation, drop, wait_for_catchup and
> > poll_query_until?
> >
> > + "Create subscription throws warning for multiple non-existent publications");
> > + "ALTER SUBSCRIPTION mysub1 ADD PUBLICATION non_existent_pub2"
> > + "ALTER SUBSCRIPTION mysub1 SET PUBLICATION non_existent_pub3"
>
> Or I would even simplify the entire tests as follows:
>
> + "CREATE SUBSCRIPTION mysub1 CONNECTION '$publisher_connstr'
> PUBLICATION mypub, non_existent_pub1"
> + "Create subscription throws warning for non-existent publication");
> >> no drop of mysub1 >>
> + "CREATE SUBSCRIPTION mysub2 CONNECTION '$publisher_connstr'
> PUBLICATION non_existent_pub1, non_existent_pub2"
> + "Create subscription throws warning for multiple non-existent publications");
> >> no drop of mysub2 >>
> + "ALTER SUBSCRIPTION mysub2 ADD PUBLICATION non_existent_pub3"
> + "Alter subscription add publication throws warning for non-existent
> publication");
> + "ALTER SUBSCRIPTION mysub2 SET PUBLICATION non_existent_pub4"
> + "Alter subscription set publication throws warning for non-existent
> publication");
> $node_subscriber->stop;
> $node_publisher->stop;
Your suggestion looks valid, I have modified it as suggested.
Additionally I have removed Create subscription with multiple
non-existent publications and changed add publication with sing
non-existent publication to add publication with multiple non-existent
publications to cover the multiple non-existent publications path.
Attached v19 patch has the changes for the same.
Regards,
Vignesh
Attachment | Content-Type | Size |
---|---|---|
v19-0001-Raise-WARNING-for-missing-publications.patch | text/x-patch | 10.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2022-03-30 16:30:24 | Re: pgsql: Add 'basebackup_to_shell' contrib module. |
Previous Message | Andres Freund | 2022-03-30 16:23:11 | Re: pgsql: Add 'basebackup_to_shell' contrib module. |