Re: Identify missing publications from publisher while create/alter subscription. - Mailing list pgsql-hackers
| From | vignesh C |
|---|---|
| Subject | Re: Identify missing publications from publisher while create/alter subscription. |
| Date | |
| Msg-id | CALDaNm3YByveb9X6EkWZzb43pE-zYqf4+36Saf9FE_mWO=mZZA@mail.gmail.com Whole thread Raw |
| In response to | Re: Identify missing publications from publisher while create/alter subscription. (Dilip Kumar <dilipbalaut@gmail.com>) |
| Responses |
Re: Identify missing publications from publisher while create/alter subscription.
(Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
|
| List | pgsql-hackers |
On Mon, Jan 25, 2021 at 3:07 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Jan 25, 2021 at 2:48 PM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
> >
> > On Mon, Jan 25, 2021 at 2:42 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On Mon, Jan 25, 2021 at 1:10 PM vignesh C <vignesh21@gmail.com> wrote:
> > > >
> > > > On Thu, Jan 21, 2021 at 10:21 PM Bharath Rupireddy
> > > > <bharath.rupireddyforpostgres@gmail.com> wrote:
> > > > >
> > > > > On Thu, Jan 21, 2021 at 6:56 PM vignesh C <vignesh21@gmail.com> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > Creating/altering subscription is successful when we specify a
> > > > > > publication which does not exist in the publisher. I felt we should
> > > > > > throw an error in this case, that will help the user to check if there
> > > > > > is any typo in the create subscription command or to create the
> > > > > > publication before the subscription is created.
> > > > > > If the above analysis looks correct, then please find a patch that
> > > > > > checks if the specified publications are present in the publisher and
> > > > > > throws an error if any of the publications is missing in the
> > > > > > publisher.
> > > > > > Thoughts?
> > > > >
> > > > > I was having similar thoughts (while working on the logical
> > > > > replication bug on alter publication...drop table behaviour) on why
> > > > > create subscription succeeds without checking the publication
> > > > > existence. I checked in documentation, to find if there's a strong
> > > > > reason for that, but I couldn't. Maybe it's because of the principle
> > > > > "first let users create subscriptions, later the publications can be
> > > > > created on the publisher system", similar to this behaviour
> > > > > "publications can be created without any tables attached to it
> > > > > initially, later they can be added".
> > > > >
> > > > > Others may have better thoughts.
> > > > >
> > > > > If we do check publication existence for CREATE SUBSCRIPTION, I think
> > > > > we should also do it for ALTER SUBSCRIPTION ... SET PUBLICATION.
> > > > >
> > > > > I wonder, why isn't dropping a publication from a list of publications
> > > > > that are with subscription is not allowed?
> > > > >
> > > > > Some comments so far on the patch:
> > > > >
> > > > > 1) I see most of the code in the new function check_publications() and
> > > > > existing fetch_table_list() is the same. Can we have a generic
> > > > > function, with maybe a flag to separate out the logic specific for
> > > > > checking publication and fetching table list from the publisher.
> > > >
> > > > I have made the common code between the check_publications and
> > > > fetch_table_list into a common function
> > > > get_appended_publications_query. I felt the rest of the code is better
> > > > off kept as it is.
> > > > The Attached patch has the changes for the same and also the change to
> > > > check publication exists during alter subscription set publication.
> > > > Thoughts?
> > > >
> > >
> > > So basically, the create subscription will throw an error if the
> > > publication does not exist. So will you throw an error if we try to
> > > drop the publication which is subscribed by some subscription? I mean
> > > basically, you are creating a dependency that if you are creating a
> > > subscription then there must be a publication that is not completely
> > > insane but then we will have to disallow dropping the publication as
> > > well. Am I missing something?
> >
> > Do you mean DROP PUBLICATION non_existent_publication;?
> >
> > Or
> >
> > Do you mean when we drop publications from a subscription?
>
> I mean it doesn’t seem right to disallow to create the subscription if
> the publisher doesn't exist, and my reasoning was even though the
> publisher exists while creating the subscription you might drop it
> later right?. So basically, now also we can create the same scenario
> that a subscription may exist for the publication which does not
> exist.
>
I would like to defer on documentation for this.
I feel we should have the behavior similar to publication tables as given below, then it will be consistent and easier for the users:
This is the behavior in case of table:
Step 1:
PUBLISHER SIDE:
create table t1(c1 int);
create table t2(c1 int);
CREATE PUBLICATION mypub1 for table t1,t2;
-- All above commands succeeds
Step 2:
SUBSCRIBER SIDE:
-- Create subscription without creating tables will result in error:
CREATE SUBSCRIPTION mysub1 CONNECTION 'dbname=source_rep host=localhost user=vignesh port=5432' PUBLICATION mypub1;
ERROR: relation "public.t2" does not exist
create table t1(c1 int);
create table t2(c1 int);
CREATE SUBSCRIPTION mysub1 CONNECTION 'dbname=source_rep host=localhost user=vignesh port=5432' PUBLICATION mypub1;
postgres=# select * from pg_subscription;
oid | subdbid | subname | subowner | subenabled | subbinary | substream | subconninfo | subslotname | subsynccommit | subpublications
-------+---------+---------+----------+------------+-----------+-----------+---------------------------------------------------------+-------------+---------------+-----------------
16392 | 13756 | mysub1 | 10 | t | f | f | dbname=source_rep host=localhost user=vignesh port=5432 | mysub1 | off | {mypub1}
(1 row)
postgres=# select *,srrelid::oid::regclass from pg_subscription_rel;
srsubid | srrelid | srsubstate | srsublsn | srrelid
---------+---------+------------+-----------+---------
16392 | 16389 | r | 0/1608BD0 | t2
16392 | 16384 | r | 0/1608BD0 | t1
(2 rows)
Step 3:
PUBLISHER:
drop table t2;
create table t3;
CREATE PUBLICATION mypub2 for table t1,t3;
Step 4:
SUBSCRIBER:
postgres=# select *,srrelid::oid::regclass from pg_subscription_rel;
srsubid | srrelid | srsubstate | srsublsn | srrelid
---------+---------+------------+-----------+---------
16392 | 16389 | r | 0/1608BD0 | t2
16392 | 16384 | r | 0/1608BD0 | t1
(2 rows)
postgres=# alter subscription mysub1 refresh publication ;
ALTER SUBSCRIPTION
-- Subscription relation will be updated.
postgres=# select *,srrelid::oid::regclass from pg_subscription_rel;
srsubid | srrelid | srsubstate | srsublsn | srrelid
---------+---------+------------+-----------+---------
16392 | 16384 | r | 0/1608BD0 | t1
(1 row)
-- Alter subscription fails while setting publication having a table that does not exist
postgres=# alter subscription mysub1 set publication mysub2;
ERROR: relation "public.t3" does not exist
To maintain consistency, we should have similar behavior in case of publication too.
If a publication which does not exist is specified during create subscription, then we should throw an error similar to step 2 behavior. Similarly if a publication which does not exist is specified during alter subscription, then we should throw an error similar to step 4 behavior. If publication is dropped after subscription is created, this should be removed when an alter subscription subname refresh publication is performed similar to step 4.
Thoughts?
pgsql-hackers by date: