PS_NITPICKS_v440001.txt

text/plain

Filename: PS_NITPICKS_v440001.txt
Type: text/plain
Part: 0
Message: Re: Introduce XID age and inactive timeout based replication slot invalidation
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 970b496..0537714 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4564,8 +4564,8 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"'  # Windows
       </term>
       <listitem>
        <para>
-        Invalidates replication slots that are inactive for longer than
-        specified amount of time. If this value is specified without units,
+        Invalidate replication slots that are inactive for longer than this
+        amount of time. If this value is specified without units,
         it is taken as seconds. A value of zero (which is default) disables
         the timeout mechanism. This parameter can only be set in
         the <filename>postgresql.conf</filename> file or on the server
@@ -4573,11 +4573,9 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"'  # Windows
        </para>
 
        <para>
-        This invalidation check happens either when the slot is acquired
-        for use or during checkpoint. The time since the slot has become
-        inactive is known from its
-        <structfield>inactive_since</structfield> value using which the
-        timeout is measured.
+        Slot invalidation due to inactivity timeout occurs during checkpoint.
+        The duration of slot inactivity is calculated using the slot's
+        <structfield>inactive_since</structfield> field value.
        </para>
 
        <para>
@@ -4585,9 +4583,8 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"'  # Windows
         applicable for slots on the standby server that are being synced
         from primary server (i.e., standby slots having
         <structfield>synced</structfield> field <literal>true</literal>).
-        Because such synced slots are typically considered not active
-        (for them to be later considered as inactive) as they don't perform
-        logical decoding to produce the changes.
+        Synced slots are always considered to be inactive because they don't
+        perform logical decoding to produce changes.
        </para>
       </listitem>
      </varlistentry>
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index acc0370..bb06592 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -551,12 +551,11 @@ ReplicationSlotName(int index, Name name)
  * An error is raised if nowait is true and the slot is currently in use. If
  * nowait is false, we sleep until the slot is released by the owning process.
  *
- * An error is raised if check_for_invalidation is true and the slot has been
+ * An error is raised if error_if_invalid is true and the slot has been
  * invalidated previously.
  */
 void
-ReplicationSlotAcquire(const char *name, bool nowait,
-					   bool check_for_invalidation)
+ReplicationSlotAcquire(const char *name, bool nowait,  bool error_if_invalid)
 {
 	ReplicationSlot *s;
 	int			active_pid;
@@ -635,11 +634,10 @@ retry:
 	MyReplicationSlot = s;
 
 	/*
-	 * Error out if the slot has been invalidated previously. Because there's
-	 * no use in acquiring the invalidated slot.
+	 * An error is raised if error_if_invalid is true and the slot has been
+	 * invalidated previously.
 	 */
-	if (check_for_invalidation &&
-		s->data.invalidated == RS_INVAL_INACTIVE_TIMEOUT)
+	if (error_if_invalid && s->data.invalidated == RS_INVAL_INACTIVE_TIMEOUT)
 	{
 		Assert(s->inactive_since > 0);
 		ereport(ERROR,
@@ -1565,6 +1563,7 @@ ReportSlotInvalidation(ReplicationSlotInvalidationCause cause,
 								 _("You might need to increase \"%s\"."), "max_slot_wal_keep_size");
 				break;
 			}
+
 		case RS_INVAL_HORIZON:
 			appendStringInfo(&err_detail, _("The slot conflicted with xid horizon %u."),
 							 snapshotConflictHorizon);
@@ -1573,6 +1572,7 @@ ReportSlotInvalidation(ReplicationSlotInvalidationCause cause,
 		case RS_INVAL_WAL_LEVEL:
 			appendStringInfoString(&err_detail, _("Logical decoding on standby requires \"wal_level\" >= \"logical\" on the primary server."));
 			break;
+
 		case RS_INVAL_INACTIVE_TIMEOUT:
 			Assert(inactive_since > 0);
 			appendStringInfo(&err_detail,
@@ -1584,6 +1584,7 @@ ReportSlotInvalidation(ReplicationSlotInvalidationCause cause,
 			appendStringInfo(err_hint,
 							 _("You might need to increase \"%s\"."), "replication_slot_inactive_timeout");
 			break;
+
 		case RS_INVAL_NONE:
 			pg_unreachable();
 	}
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index 431cc08..5678e1a 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -253,7 +253,7 @@ extern void ReplicationSlotAlter(const char *name, const bool *failover,
 								 const bool *two_phase);
 
 extern void ReplicationSlotAcquire(const char *name, bool nowait,
-								   bool check_for_invalidation);
+								   bool error_if_invalid);
 extern void ReplicationSlotRelease(void);
 extern void ReplicationSlotCleanup(bool synced_only);
 extern void ReplicationSlotSave(void);