Re: Single transaction in the tablesync worker?

On Mon, Feb 1, 2021 at 11:23 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Mon, Feb 1, 2021 at 3:44 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Mon, Feb 1, 2021 at 9:38 AM Peter Smith <smithpb2250(at)gmail(dot)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.