On Wed, Nov 22, 2023 at 2:28 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Wed, Nov 22, 2023 at 2:12 PM Bharath Rupireddy > <bharath.rupireddyforpostg...@gmail.com> wrote: > > > > While looking at the use of session_replication_state, I noticed that > > ReplicationOriginLock is acquired in ReplicationOriginExitCleanup() > > even if session_replication_state is reset to NULL by > > replorigin_session_reset(). Why can't there be a lockless exit path > > something like [1] similar to > > replorigin_session_reset() which checks session_replication_state == > > NULL without a lock? > > > > I don't see any problem with such a check but not sure of the benefit > of doing so either.
It avoids an unnecessary lock acquisition and release when session_replication_state is already reset by replorigin_session_reset() before reaching ReplicationOriginExitCleanup(). A patch something like [1] and a run of subscription tests shows that 153 times the lock acquisition and release can be avoided. ubuntu:~/postgres/src/test/subscription$ grep -ir "with session_replication_state NULL" . | wc -l 153 ubuntu:~/postgres/src/test/subscription$ grep -ir "with session_replication_state not NULL" . | wc -l 174 [1] diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c index 460e3dcc38..dd3824bd27 100644 --- a/src/backend/replication/logical/origin.c +++ b/src/backend/replication/logical/origin.c @@ -1056,6 +1056,11 @@ ReplicationOriginExitCleanup(int code, Datum arg) { ConditionVariable *cv = NULL; + if (session_replication_state == NULL) + elog(LOG, "In ReplicationOriginExitCleanup() with session_replication_state NULL"); + else + elog(LOG, "In ReplicationOriginExitCleanup() with session_replication_state not NULL"); + LWLockAcquire(ReplicationOriginLock, LW_EXCLUSIVE); if (session_replication_state != NULL && -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com