v78_amit.1.patch.txt

text/plain

Filename: v78_amit.1.patch.txt
Type: text/plain
Part: 0
Message: Re: Introduce XID age and inactive timeout based replication slot invalidation
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 4ff66f0dd5..476b8e5355 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -606,6 +606,11 @@ retry:
 		if (!nowait)
 			ConditionVariablePrepareToSleep(&s->active_cv);
 
+		/*
+		 * It is important to reset the inactive_since under spinlock here to
+		 * avoid race conditions with slot invalidation. See comments related
+		 * to inactive_since in InvalidatePossiblyObsoleteSlot.
+		 */
 		SpinLockAcquire(&s->mutex);
 		if (s->active_pid == 0)
 			s->active_pid = MyProcPid;
@@ -1641,11 +1646,10 @@ DetermineSlotInvalidationCause(uint32 possible_causes, ReplicationSlot *s,
 
 	if (possible_causes & RS_INVAL_HORIZON)
 	{
-		if (SlotIsLogical(s) &&
 		/* invalid DB oid signals a shared relation */
+		if (SlotIsLogical(s) &&
 			(dboid == InvalidOid || dboid == s->data.database))
 		{
-
 			if (TransactionIdIsValid(initial_effective_xmin) &&
 				TransactionIdPrecedesOrEquals(initial_effective_xmin,
 											  snapshotConflictHorizon))
@@ -1757,11 +1761,13 @@ InvalidatePossiblyObsoleteSlot(uint32 possible_causes,
 			 * The slot's mutex will be released soon, and it is possible that
 			 * those values change since the process holding the slot has been
 			 * terminated (if any), so record them here to ensure that we
-			 * would report the correct invalidation cause. No need to record
-			 * inactive_since for the idle_timeout case here, as an already
-			 * inactive slot's inactive_since can only be reset under a mutex
-			 * in ReplicationSlotAcquire(), and an inactive slot can be
-			 * invalidated immediately without releasing the spinlock.
+			 * would report the correct invalidation cause.
+			 *
+			 * Unlike others slot's inactive_since can't be changed once it is
+			 * acquired till it gets released or the process owning it gets
+			 * terminated. The slot remains active till some process owns it.
+			 * So, the inactive slot can only be invalidated immediately
+			 * without being terminated.
 			 */
 			if (!terminated)
 			{