On Mon, Feb 1, 2021 at 11:23 AM Peter Smith <smithpb2...@gmail.com> wrote: > > On Mon, Feb 1, 2021 at 3:44 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Mon, Feb 1, 2021 at 9:38 AM Peter Smith <smithpb2...@gmail.com> wrote: > > > > No, there is a functionality change as well. The way we have code in > > v22 can easily lead to a problem when we have dropped the slots but > > get an error while removing origins or an entry from subscription rel. > > In such cases, we won't be able to rollback the drop of slots but the > > other database operations will be rolled back. This is the reason we > > have to drop the slots at the end. We need to ensure the same thing > > for AlterSubscription_refresh. Does this make sense now? > > > > OK. >
I have updated the patch to display WARNING for each of the tablesync slots during DropSubscription. As discussed, I have moved the drop slot related code towards the end in AlterSubscription_refresh. Apart from this, I have fixed one more issue in tablesync code where in after catching the exception we were not clearing the transaction state on the publisher, see changes in LogicalRepSyncTableStart. I have also fixed other comments raised by you. Additionally, I have removed the test because it was creating the same name slot as the tablesync worker and tablesync worker removed the same due to new logic in LogicalRepSyncStart. Earlier, it was not failing because of the bug in that code which I have fixed in the attached. I wonder whether we should restrict creating slots with prefix pg_ because we are internally creating slots with those names? I think this was a problem previously also. We already prohibit it for few other objects like origins, schema, etc., see the usage of IsReservedName. -- With Regards, Amit Kapila.
v24-0001-Tablesync-Solution1.patch
Description: Binary data
