Re: pg_replication_origin_drop API potential race condition - Mailing list pgsql-hackers

From Peter Smith
Subject Re: pg_replication_origin_drop API potential race condition
Date
Msg-id CAHut+PtuNKnacxYsLYpnWO7d4ysJciyiD70HtZTLSte+XMMVjQ@mail.gmail.com
Whole thread Raw
In response to Re: pg_replication_origin_drop API potential race condition  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: pg_replication_origin_drop API potential race condition
List pgsql-hackers
On Fri, Feb 5, 2021 at 6:02 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Feb 5, 2021 at 9:46 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > PSA patch updated per above suggestions.
> >
>
> Thanks, I have tested your patch and before the patch, I was getting
> errors like "tuple concurrently deleted" or "cache lookup failed for
> replication origin with oid 1" and after the patch, I am getting
> "replication origin "origin-1" does not exist" which is clearly better
> and user-friendly.
>
> Before Patch
> postgres=# select pg_replication_origin_drop('origin-1');
> ERROR:  tuple concurrently deleted
> postgres=# select pg_replication_origin_drop('origin-1');
> ERROR:  cache lookup failed for replication origin with oid 1
>
> After Patch
> postgres=# select pg_replication_origin_drop('origin-1');
> ERROR:  replication origin "origin-1" does not exist
>
> I wonder why you haven't changed the usage of the existing
> replorigin_drop in the code? I have changed the same, added few
> comments, ran pgindent, and updated the commit message in the
> attached.

You are right.

The goal of this patch was to fix pg_replication_origin_drop, but
while focussed on fixing that, I forgot the same call pattern was also
in the DropSubscription.

>
> I am not completely whether we should retire replorigin_drop or just
> keep it for backward compatibility? What do you think? Anybody else
> has any opinion?

It is still good code, but just not being used atm.

I don't know what is the PG convention for dead code - to remove it
immedaitely at first sight, or to leave it lying around if it still
might have future usefulness?
Personally, I would leave it, if only because it seems a less radical
change from the current HEAD code to keep the existing function
signature.

------
Kind Regards,
Peter Smith.
Fujitsu Australia



pgsql-hackers by date:

Previous
From: Greg Nancarrow
Date:
Subject: Re: Parallel INSERT (INTO ... SELECT ...)
Next
From: Amit Kapila
Date:
Subject: Re: Single transaction in the tablesync worker?