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

From Alvaro Herrera
Subject Re: pg_replication_origin_drop API potential race condition
Date
Msg-id 20210208180037.GA16704@alvherre.pgsql
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
> +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.

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.

-- 
Álvaro Herrera       Valdivia, Chile
"On the other flipper, one wrong move and we're Fatal Exceptions"
(T.U.X.: Term Unit X  - http://www.thelinuxreview.com/TUX/)



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Prevent printing "next step instructions" in initdb and pg_upgrade
Next
From: Jacob Champion
Date:
Subject: Re: Allow matching whole DN from a client certificate