Re: pg_replication_origin_drop API potential race condition

> +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/)