On Tue, Feb 9, 2021 at 9:27 AM Amit Kapila <amit.kapil...@gmail.com> wrote:
>
> On Mon, Feb 8, 2021 at 11:30 PM Alvaro Herrera <alvhe...@alvh.no-ip.org>
> wrote:
> >
> > > +void
> > > +replorigin_drop_by_name(char *name, bool missing_ok, bool nowait)
> > > +{
> > > + RepOriginId roident;
> > > + Relation rel;
> > > +
> > > + Assert(IsTransactionState());
> > > +
> > > + /*
> > > + * To interlock against concurrent drops, we hold ExclusiveLock on
> > > + * pg_replication_origin throughout this function.
> > > + */
> >
> > This comment is now wrong though; should s/throughout.*/till xact commit/
> > to reflect the new reality.
> >
>
> Right, I'll fix in the next version.
>Fixed in the attached. > > I do wonder if this is going to be painful in some way, since the lock > > is now going to be much longer-lived. My impression is that it's okay, > > since dropping an origin is not a very frequent occurrence. It is going > > to block pg_replication_origin_advance() with *any* origin, which > > acquires RowExclusiveLock on the same relation. If this is a problem, > > then we could use LockSharedObject() in both places (and make it last > > till end of xact for the case of deletion), instead of holding this > > catalog-level lock till end of transaction. > > > > IIUC, you are suggesting to use lock for the particular origin instead > of locking the corresponding catalog table in functions > pg_replication_origin_advance and replorigin_drop_by_name. If so, I > don't see any problem with the same > I think it won't be that straightforward as we don't have origin_id. So what we instead need to do is first to acquire a lock on ReplicationOriginRelationId, get the origin_id, lock the specific origin and then re-check if the origin still exists. I feel some similar changes might be required in pg_replication_origin_advance. Now, we can do this optimization if we want but I am not sure if origin_drop would be a frequent enough operation that we add such an optimization. For now, I have added a note in the comments so that if we find any such use case we can implement such optimization in the future. What do you think? -- With Regards, Amit Kapila.
v6-0001-Make-pg_replication_origin_drop-safe-against-conc.patch
Description: Binary data
