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? Regards, Vignesh EnterpriseDB: http://www.enterprisedb.com |
On Thu, Jan 21, 2021 at 6:56 PM vignesh C <[hidden email]> 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. 2) Can't we know whether the publications exist on the publisher with the existing (or modifying it a bit if required) query in fetch_table_list(), so that we can avoid making another connection to the publisher system from the subscriber? 3) If multiple publications are specified in the CREATE SUBSCRIPTION query, IIUC, with your patch, the query fails even if at least one of the publications doesn't exist. Should we throw a warning in this case and allow the subscription be created for other existing publications? With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com |
On Fri, 22 Jan 2021 at 00:51, Bharath Rupireddy <[hidden email]> wrote: > On Thu, Jan 21, 2021 at 6:56 PM vignesh C <[hidden email]> 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. > Agreed. Current patch do not check publication existence for ALTER SUBSCRIPTION ... SET PUBLICATION ... WITH (refresh = false). > 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. +1 > 2) Can't we know whether the publications exist on the publisher with > the existing (or modifying it a bit if required) query in > fetch_table_list(), so that we can avoid making another connection to > the publisher system from the subscriber? IIUC, the patch does not make another connection, it just execute a new query in already connection. If we want to check publication existence for ALTER SUBSCRIPTION ... SET PUBLICATION ... WITH (refresh = false) we should make another connection. > 3) If multiple publications are specified in the CREATE SUBSCRIPTION > query, IIUC, with your patch, the query fails even if at least one of > the publications doesn't exist. Should we throw a warning in this case > and allow the subscription be created for other existing > publications? > +1. If all the publications do not exist, we should throw an error. -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd. |
On Fri, Jan 22, 2021 at 10:14 AM japin <[hidden email]> wrote:
> > 2) Can't we know whether the publications exist on the publisher with > > the existing (or modifying it a bit if required) query in > > fetch_table_list(), so that we can avoid making another connection to > > the publisher system from the subscriber? > > IIUC, the patch does not make another connection, it just execute a new > query in already connection. If we want to check publication existence > for ALTER SUBSCRIPTION ... SET PUBLICATION ... WITH (refresh = false) > we should make another connection. Actually, I meant that we can avoid submitting another SQL query to the publisher if we could manage to submit a single query that first checks if a given publication exists in pg_publication and if yes returns the tables associated with it from pg_publication_tables. Can we modify the existing query in fetch_table_list that gets only the table list from pg_publcation_tables to see if the given publication exists in the pg_publication? Yes you are right, if we were to check the existence of publications provided with ALTER SUBSCRIPTION statements, we need to do walrcv_connect, walrcv_exec. We could just call a common function from there. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com |
On Fri, Jan 22, 2021 at 12:14 PM Bharath Rupireddy
<[hidden email]> wrote: > > On Fri, Jan 22, 2021 at 10:14 AM japin <[hidden email]> wrote: > > > 2) Can't we know whether the publications exist on the publisher with > > > the existing (or modifying it a bit if required) query in > > > fetch_table_list(), so that we can avoid making another connection to > > > the publisher system from the subscriber? > > > > IIUC, the patch does not make another connection, it just execute a new > > query in already connection. If we want to check publication existence > > for ALTER SUBSCRIPTION ... SET PUBLICATION ... WITH (refresh = false) > > we should make another connection. > > Actually, I meant that we can avoid submitting another SQL query to > the publisher if we could manage to submit a single query that first > checks if a given publication exists in pg_publication and if yes > returns the tables associated with it from pg_publication_tables. Can > we modify the existing query in fetch_table_list that gets only the > table list from pg_publcation_tables to see if the given publication > exists in the pg_publication? > that we might need some function/procedure to do this. I felt this approach is more simpler and chose this approach. Thoughts? > Yes you are right, if we were to check the existence of publications > provided with ALTER SUBSCRIPTION statements, we need to do > walrcv_connect, walrcv_exec. We could just call a common function from > there. > Yes I agree this should be done in ALTER SUBSCRIPTION SET PUBLICATION case also, currently we do if refresh is enabled, it should also be done in ALTER SUBSCRIPTION mysub SET PUBLICATION mypub WITH (REFRESH = FALSE) also. I will include this in my next version of the patch. Regards, Vignesh EnterpriseDB: http://www.enterprisedb.com |
In reply to this post by japin
On Fri, Jan 22, 2021 at 10:14 AM japin <[hidden email]> wrote:
> > > On Fri, 22 Jan 2021 at 00:51, Bharath Rupireddy <[hidden email]> wrote: > > On Thu, Jan 21, 2021 at 6:56 PM vignesh C <[hidden email]> 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. > > > > Agreed. Current patch do not check publication existence for > ALTER SUBSCRIPTION ... SET PUBLICATION ... WITH (refresh = false). > > > 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. > > +1 > > > 2) Can't we know whether the publications exist on the publisher with > > the existing (or modifying it a bit if required) query in > > fetch_table_list(), so that we can avoid making another connection to > > the publisher system from the subscriber? > > IIUC, the patch does not make another connection, it just execute a new > query in already connection. If we want to check publication existence > for ALTER SUBSCRIPTION ... SET PUBLICATION ... WITH (refresh = false) > we should make another connection. > > > 3) If multiple publications are specified in the CREATE SUBSCRIPTION > > query, IIUC, with your patch, the query fails even if at least one of > > the publications doesn't exist. Should we throw a warning in this case > > and allow the subscription be created for other existing > > publications? > > > > +1. If all the publications do not exist, we should throw an error. I also felt if any of the publications are not there, we should throw an error. Regards, Vignesh EnterpriseDB: http://www.enterprisedb.com |
In reply to this post by Bharath Rupireddy
On Thu, Jan 21, 2021 at 10:21 PM Bharath Rupireddy
<[hidden email]> wrote: > > On Thu, Jan 21, 2021 at 6:56 PM vignesh C <[hidden email]> 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. 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? Regards, Vignesh EnterpriseDB: http://www.enterprisedb.com |
On Mon, Jan 25, 2021 at 1:10 PM vignesh C <[hidden email]> wrote:
> > On Thu, Jan 21, 2021 at 10:21 PM Bharath Rupireddy > <[hidden email]> wrote: > > > > On Thu, Jan 21, 2021 at 6:56 PM vignesh C <[hidden email]> 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? -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com |
On Mon, Jan 25, 2021 at 2:42 PM Dilip Kumar <[hidden email]> wrote:
> > On Mon, Jan 25, 2021 at 1:10 PM vignesh C <[hidden email]> wrote: > > > > On Thu, Jan 21, 2021 at 10:21 PM Bharath Rupireddy > > <[hidden email]> wrote: > > > > > > On Thu, Jan 21, 2021 at 6:56 PM vignesh C <[hidden email]> 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? If yes, do we have a way to drop a publication from the subscription? See below one of my earlier questions on this. "I wonder, why isn't dropping a publication from a list of publications that are with subscription is not allowed?" At least, I see no ALTER SUBSCRIPTION ... DROP PUBLICATION mypub1 or something similar? With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com |
On Mon, Jan 25, 2021 at 2:48 PM Bharath Rupireddy
<[hidden email]> wrote: > > On Mon, Jan 25, 2021 at 2:42 PM Dilip Kumar <[hidden email]> wrote: > > > > On Mon, Jan 25, 2021 at 1:10 PM vignesh C <[hidden email]> wrote: > > > > > > On Thu, Jan 21, 2021 at 10:21 PM Bharath Rupireddy > > > <[hidden email]> wrote: > > > > > > > > On Thu, Jan 21, 2021 at 6:56 PM vignesh C <[hidden email]> 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. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com |
On Mon, Jan 25, 2021 at 3:07 PM Dilip Kumar <[hidden email]> wrote:
> > > 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. Yes, the above scenario can be created even now. If a publication is dropped in the publisher system, then it will not replicate/publish the changes for that publication (publication_invalidation_cb, rel_sync_cache_publication_cb, LoadPublications in get_rel_sync_entry), so subscriber doesn't receive them. But the subscription can still contain that dropped publication in it's list of publications. The patch proposed in this thread, just checks while creation/altering of the subscription on the subscriber system whether or not the publication exists on the publisher system. This is one way dependency. But given the above scenario, there can exist another dependency i.e. publisher dropping the publisher at any time. So to make it a complete solution i.e. not allowing non-existent publications from the list of publications in the subscription, we need to detect when the publications are dropped in the publisher and we should, may be on a next connection to the subscriber, also look at the subscription for that dropped publication, if exists remove it. But that's an overkill and impractical I feel. Thoughts? I also feel the best way to remove the confusion is to document why we allow creating subscriptions even when the specified publications don't exist on the publisher system? Thoughts? With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com |
On Mon, Jan 25, 2021 at 3:38 PM Bharath Rupireddy
<[hidden email]> wrote: > > On Mon, Jan 25, 2021 at 3:07 PM Dilip Kumar <[hidden email]> wrote: > > > > 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. > > Yes, the above scenario can be created even now. If a publication is > dropped in the publisher system, then it will not replicate/publish > the changes for that publication (publication_invalidation_cb, > rel_sync_cache_publication_cb, LoadPublications in > get_rel_sync_entry), so subscriber doesn't receive them. But the > subscription can still contain that dropped publication in it's list > of publications. > > The patch proposed in this thread, just checks while creation/altering > of the subscription on the subscriber system whether or not the > publication exists on the publisher system. This is one way > dependency. But given the above scenario, there can exist another > dependency i.e. publisher dropping the publisher at any time. So to > make it a complete solution i.e. not allowing non-existent > publications from the list of publications in the subscription, we > need to detect when the publications are dropped in the publisher and > we should, may be on a next connection to the subscriber, also look at > the subscription for that dropped publication, if exists remove it. > But that's an overkill and impractical I feel. Thoughts? > > I also feel the best way to remove the confusion is to document why we > allow creating subscriptions even when the specified publications > don't exist on the publisher system? Thoughts? Yes, that was my point that there is no point in solving it in some cases and it can exist in other cases. So I am fine with documenting the behavior. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com |
In reply to this post by Bharath Rupireddy
On Mon, 25 Jan 2021 at 17:18, Bharath Rupireddy <[hidden email]> wrote: > On Mon, Jan 25, 2021 at 2:42 PM Dilip Kumar <[hidden email]> wrote: >> >> On Mon, Jan 25, 2021 at 1:10 PM vignesh C <[hidden email]> wrote: >> > >> > On Thu, Jan 21, 2021 at 10:21 PM Bharath Rupireddy >> > <[hidden email]> wrote: >> > > >> > > On Thu, Jan 21, 2021 at 6:56 PM vignesh C <[hidden email]> 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? If yes, do > we have a way to drop a publication from the subscription? See below > one of my earlier questions on this. > "I wonder, why isn't dropping a publication from a list of > publications that are with subscription is not allowed?" > At least, I see no ALTER SUBSCRIPTION ... DROP PUBLICATION mypub1 or > something similar? > Why we do not support ALTER SUBSCRIPTION...ADD/DROP PUBLICATION? When we have multiple publications in subscription, but I want to add/drop a single publication, it is conveient. The ALTER SUBSCRIPTION...SET PUBLICATION... should supply the completely publications. Sorry, this question is unrelated with this subject. -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd. |
On Mon, Jan 25, 2021 at 5:18 PM japin <[hidden email]> wrote:
> > Do you mean when we drop publications from a subscription? If yes, do > > we have a way to drop a publication from the subscription? See below > > one of my earlier questions on this. > > "I wonder, why isn't dropping a publication from a list of > > publications that are with subscription is not allowed?" > > At least, I see no ALTER SUBSCRIPTION ... DROP PUBLICATION mypub1 or > > something similar? > > > > Why we do not support ALTER SUBSCRIPTION...ADD/DROP PUBLICATION? When we > have multiple publications in subscription, but I want to add/drop a single > publication, it is conveient. The ALTER SUBSCRIPTION...SET PUBLICATION... > should supply the completely publications. Looks like the way to drop/add publication from the list of publications in subscription requires users to specify all the list of publications currently exists +/- the new publication that needs to be added/dropped: CREATE SUBSCRIPTION mysub1 CONNECTION 'host=localhost port=5432 dbname=postgres' PUBLICATION mypub1, mypub2, mypu3, mypub4, mypub5; postgres=# select subpublications from pg_subscription; subpublications ------------------------------------- {mypub1,mypub2,mypu3,mypub4,mypub5} (1 row) Say, I want to drop mypub4: ALTER SUBSCRIPTION mysub1 SET PUBLICATION mypub1, mypub2, mypu3, mypub5; postgres=# select subpublications from pg_subscription; subpublications ------------------------------ {mypub1,mypub2,mypu3,mypub5} Say, I want toa dd mypub4 and mypub6: ALTER SUBSCRIPTION mysub1 SET PUBLICATION mypub1, mypub2, mypu3, mypub5, mypub4, mypub6; postgres=# select subpublications from pg_subscription; subpublications -------------------------------------------- {mypub1,mypub2,mypu3,mypub5,mypub4,mypub6} (1 row) It will be good to have something like: ALTER SUBSCRIPTION mysub1 ADD PUBLICATION mypub1, mypub3; which will the publications to subscription if not added previously. ALTER SUBSCRIPTION mysub1 DROP PUBLICATION mypub1, mypub3; which will drop the publications from subscription if they exist in the subscription's list of publications. But I'm really not sure why the above syntax was not added earlier. We may be missing something here. > Sorry, this question is unrelated with this subject. Yes, IMO it can definitely be discussed in another thread. It will be good to get a separate opinion for this. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com |
In reply to this post by japin
On Mon, Jan 25, 2021 at 5:18 PM japin <[hidden email]> wrote:
> > > On Mon, 25 Jan 2021 at 17:18, Bharath Rupireddy <[hidden email]> wrote: > > On Mon, Jan 25, 2021 at 2:42 PM Dilip Kumar <[hidden email]> wrote: > >> > >> On Mon, Jan 25, 2021 at 1:10 PM vignesh C <[hidden email]> wrote: > >> > > >> > On Thu, Jan 21, 2021 at 10:21 PM Bharath Rupireddy > >> > <[hidden email]> wrote: > >> > > > >> > > On Thu, Jan 21, 2021 at 6:56 PM vignesh C <[hidden email]> 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? If yes, do > > we have a way to drop a publication from the subscription? See below > > one of my earlier questions on this. > > "I wonder, why isn't dropping a publication from a list of > > publications that are with subscription is not allowed?" > > At least, I see no ALTER SUBSCRIPTION ... DROP PUBLICATION mypub1 or > > something similar? > > > > Why we do not support ALTER SUBSCRIPTION...ADD/DROP PUBLICATION? When we > have multiple publications in subscription, but I want to add/drop a single > publication, it is conveient. The ALTER SUBSCRIPTION...SET PUBLICATION... > should supply the completely publications. > > Sorry, this question is unrelated with this subject. Please start a new thread for this, let's discuss this separately. Regards, Vignesh EnterpriseDB: http://www.enterprisedb.com |
In reply to this post by Dilip Kumar-2
On Mon, Jan 25, 2021 at 3:07 PM Dilip Kumar <[hidden email]> wrote: > > On Mon, Jan 25, 2021 at 2:48 PM Bharath Rupireddy > <[hidden email]> wrote: > > > > On Mon, Jan 25, 2021 at 2:42 PM Dilip Kumar <[hidden email]> wrote: > > > > > > On Mon, Jan 25, 2021 at 1:10 PM vignesh C <[hidden email]> wrote: > > > > > > > > On Thu, Jan 21, 2021 at 10:21 PM Bharath Rupireddy > > > > <[hidden email]> wrote: > > > > > > > > > > On Thu, Jan 21, 2021 at 6:56 PM vignesh C <[hidden email]> 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? |
In reply to this post by Bharath Rupireddy
On Mon, 25 Jan 2021 at 21:55, Bharath Rupireddy <[hidden email]> wrote: > On Mon, Jan 25, 2021 at 5:18 PM japin <[hidden email]> wrote: >> > Do you mean when we drop publications from a subscription? If yes, do >> > we have a way to drop a publication from the subscription? See below >> > one of my earlier questions on this. >> > "I wonder, why isn't dropping a publication from a list of >> > publications that are with subscription is not allowed?" >> > At least, I see no ALTER SUBSCRIPTION ... DROP PUBLICATION mypub1 or >> > something similar? >> > >> >> Why we do not support ALTER SUBSCRIPTION...ADD/DROP PUBLICATION? When we >> have multiple publications in subscription, but I want to add/drop a single >> publication, it is conveient. The ALTER SUBSCRIPTION...SET PUBLICATION... >> should supply the completely publications. > > Looks like the way to drop/add publication from the list of > publications in subscription requires users to specify all the list of > publications currently exists +/- the new publication that needs to be > added/dropped: > > CREATE SUBSCRIPTION mysub1 CONNECTION 'host=localhost port=5432 > dbname=postgres' PUBLICATION mypub1, mypub2, mypu3, mypub4, mypub5; > postgres=# select subpublications from pg_subscription; > subpublications > ------------------------------------- > {mypub1,mypub2,mypu3,mypub4,mypub5} > (1 row) > > Say, I want to drop mypub4: > > ALTER SUBSCRIPTION mysub1 SET PUBLICATION mypub1, mypub2, mypu3, mypub5; > postgres=# select subpublications from pg_subscription; > subpublications > ------------------------------ > {mypub1,mypub2,mypu3,mypub5} > > Say, I want toa dd mypub4 and mypub6: > ALTER SUBSCRIPTION mysub1 SET PUBLICATION mypub1, mypub2, mypu3, > mypub5, mypub4, mypub6; > postgres=# select subpublications from pg_subscription; > subpublications > -------------------------------------------- > {mypub1,mypub2,mypu3,mypub5,mypub4,mypub6} > (1 row) > > It will be good to have something like: > > ALTER SUBSCRIPTION mysub1 ADD PUBLICATION mypub1, mypub3; which will > the publications to subscription if not added previously. > > ALTER SUBSCRIPTION mysub1 DROP PUBLICATION mypub1, mypub3; which will > drop the publications from subscription if they exist in the > subscription's list of publications. > > But I'm really not sure why the above syntax was not added earlier. We > may be missing something here. > >> Sorry, this question is unrelated with this subject. > > Yes, IMO it can definitely be discussed in another thread. It will be > good to get a separate opinion for this. > I started a new thread [1] for this, please have a look. [1] - https://www.postgresql.org/message-id/MEYP282MB166939D0D6C480B7FBE7EFFBB6BC0@... -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd. |
In reply to this post by vignesh C
On Mon, Jan 25, 2021 at 10:32 PM vignesh C <[hidden email]> wrote:
> > 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? IIUC, your idea is to check if the publications (that are associated with a subscription) are present in the publisher or not during ALTER SUBSCRIPTION ... REFRESH PUBLICATION;. If that's the case, then I have scenario: 1) subscription is created with pub1, pub2 and assume both the publications are present in the publisher 2) pub1 and pub2 tables data is replicated properly 3) pub2 is dropped on the publisher 4) run alter subscription .. refresh publication on the subscriber, so that the pub2 tables will be removed from the subscriber 5) for some reason, user creates pub2 again on the publisher and want to replicated some tables 6) run alter subscription .. refresh publication on the subscriber, so that the pub2 tables will be added to the subscriber table list Now, if we remove the dropped publication pub2 in step 4 from the subscription list(as per your above analysis and suggestion), then after step 5, users will need to add the publication pub2 to the subscription again. I feel this is a change in the current behaviour. The existing behaviour on master doesn't mandate this as the dropped publications are not removed from the subscription list at all. To not mandate any new behaviour, I would suggest to have a new option for ALTER SUBSCRIPTION ... REFRESH PUBLICATION WITH (remove_dropped_publications = false). The new option remove_dropped_publications will have a default value false, when set to true it will check if the publications that are present in the subscription list are actually existing on the publisher or not, if not remove them from the list. And also in the documentation we need to clearly mention the consequence of this new option setting to true, that is, the dropped publications if created again will need to be added to the subscription list again. Thoughts? With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com |
On Wed, Feb 3, 2021, at 2:13 AM, Bharath Rupireddy wrote:
Postgres implemented a replication mechanism that is decoupled which means that both parties can perform "uncoordinated" actions. As you shown, the CREATE SUBSCRIPTION informing a non-existent publication is one of them. I think that being permissive in some situations can prevent you from painting yourself into a corner. Even if you try to be strict on the subscriber side, the other side (publisher) can impose you additional complexity. You are arguing that in the initial phase, the CREATE SUBSCRIPTION has a strict mechanism. It is a fair point. However, impose this strictness for the other SUBSCRIPTION commands should be carefully forethought. If we go that route, I suggest to have the current behavior as an option. The reasons are: (i) it is backward-compatible, (ii) it allows us some known flexibility (non-existent publication), and (iii) it would probably allow us to fix a scenario created by the strict mode. This strict mode can be implemented via new parameter validate_publication (ENOCAFFEINE to propose a better name) that checks if the publication is available when you executed the CREATE SUBSCRIPTION. Similar parameter can be used in ALTER SUBSCRIPTION ... SET PUBLICATION and ALTER SUBSCRIPTION ... REFRESH PUBLICATION.
REFRESH PUBLICATION is not the right command to remove publications. There is a command for it: ALTER SUBSCRIPTION ... SET PUBLICATION. The other alternative is to document that non-existent publication names can be in the subscription catalog and it is ignored while executing SUBSCRIPTION commands. You could possibly propose a NOTICE/WARNING that informs the user that the SUBSCRIPTION command contains non-existent publication. |
On Wed, Mar 3, 2021 at 8:59 AM Euler Taveira <[hidden email]> wrote:
> > On Wed, Feb 3, 2021, at 2:13 AM, Bharath Rupireddy wrote: > > On Mon, Jan 25, 2021 at 10:32 PM vignesh C <[hidden email]> wrote: > > > 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? > > Postgres implemented a replication mechanism that is decoupled which means that > both parties can perform "uncoordinated" actions. As you shown, the CREATE > SUBSCRIPTION informing a non-existent publication is one of them. I think that > being permissive in some situations can prevent you from painting yourself into > a corner. Even if you try to be strict on the subscriber side, the other side > (publisher) can impose you additional complexity. > > You are arguing that in the initial phase, the CREATE SUBSCRIPTION has a strict > mechanism. It is a fair point. However, impose this strictness for the other > SUBSCRIPTION commands should be carefully forethought. If we go that route, I > suggest to have the current behavior as an option. The reasons are: (i) it is > backward-compatible, (ii) it allows us some known flexibility (non-existent > publication), and (iii) it would probably allow us to fix a scenario created by > the strict mode. This strict mode can be implemented via new parameter > validate_publication (ENOCAFFEINE to propose a better name) that checks if the > publication is available when you executed the CREATE SUBSCRIPTION. Similar > parameter can be used in ALTER SUBSCRIPTION ... SET PUBLICATION and ALTER > SUBSCRIPTION ... REFRESH PUBLICATION. IIUC the validate_publication is kind of the feature enable/disable flag. With the default value being false, when set to true, it checks whether the publications exists or not while CREATE/ALTER SUBSCRIPTION and throw error. If I'm right, then the validate_publication option looks good to me. So, whoever wants to impose strict restrictions to CREATE/ALTER subscription can set it to true. All others will not see any new behaviour or such. > To not mandate any new behaviour, I would suggest to have a new option > for ALTER SUBSCRIPTION ... REFRESH PUBLICATION WITH > (remove_dropped_publications = false). The new option > remove_dropped_publications will have a default value false, when set > to true it will check if the publications that are present in the > subscription list are actually existing on the publisher or not, if > not remove them from the list. And also in the documentation we need > to clearly mention the consequence of this new option setting to true, > that is, the dropped publications if created again will need to be > added to the subscription list again. > > REFRESH PUBLICATION is not the right command to remove publications. There is a > command for it: ALTER SUBSCRIPTION ... SET PUBLICATION. > > The other alternative is to document that non-existent publication names can be > in the subscription catalog and it is ignored while executing SUBSCRIPTION > commands. You could possibly propose a NOTICE/WARNING that informs the user > that the SUBSCRIPTION command contains non-existent publication. I think, we can also have validate_publication option allowed for ALTER SUBSCRIPTION SET PUBLICATION and REFRESH PUBLICATION commands with the same behaviour i.e. error out when specified publications don't exist in the publisher. Thoughts? With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com |
Free forum by Nabble | Edit this page |