On Mon, Mar 25, 2024 at 9:48 AM Amit Kapila <amit.kapil...@gmail.com> wrote:
>
>
> Such a test looks reasonable but shall we add equal to in the second
> part of the test (like '$last_inactive_time'::timestamptz >=
> > '$slot_creation_time'::timestamptz;). This is just to be sure that even if
> > the test ran fast enough to give the same time, the test shouldn't fail. I
> > think it won't matter for correctness as well.
>
Apart from this, I have made minor changes in the comments. See and
let me know what you think of attached.
--
With Regards,
Amit Kapila.
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 2b36b5fef1..5f4165a945 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -2529,8 +2529,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
</para>
<para>
The time at which the slot became inactive.
- <literal>NULL</literal> if the slot is currently actively being
- used.
+ <literal>NULL</literal> if the slot is currently being used.
</para></entry>
</row>
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 0f48d6dc7c..77cb633812 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -623,7 +623,7 @@ retry:
if (SlotIsLogical(s))
pgstat_acquire_replslot(s);
- /* The slot is active by now, so reset the last inactive time. */
+ /* Reset the last inactive time as the slot is active now. */
SpinLockAcquire(&s->mutex);
s->last_inactive_time = 0;
SpinLockRelease(&s->mutex);
@@ -687,8 +687,8 @@ ReplicationSlotRelease(void)
}
/*
- * Set the last inactive time after marking slot inactive. We get
current
- * time beforehand to avoid system call while holding the lock.
+ * Set the last inactive time after marking the slot inactive. We get
the
+ * current time beforehand to avoid a system call while holding the
lock.
*/
now = GetCurrentTimestamp();
@@ -2363,9 +2363,9 @@ RestoreSlotFromDisk(const char *name)
slot->active_pid = 0;
/*
- * We set last inactive time after loading the slot from the
disk into
- * memory. Whoever acquires the slot i.e. makes the slot active
will
- * anyway reset it.
+ * We set the last inactive time after loading the slot from
the disk
+ * into memory. Whoever acquires the slot i.e. makes the slot
active
+ * will reset it.
*/
slot->last_inactive_time = GetCurrentTimestamp();
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index 2f18433ecc..eefd7abd39 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -202,7 +202,7 @@ typedef struct ReplicationSlot
*/
XLogRecPtr last_saved_confirmed_flush;
- /* The time at which this slot become inactive */
+ /* The time at which this slot becomes inactive */
TimestampTz last_inactive_time;
} ReplicationSlot;
diff --git a/src/test/recovery/t/019_replslot_limit.pl
b/src/test/recovery/t/019_replslot_limit.pl
index bff84cc9c4..81bd36f5d8 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -411,7 +411,7 @@ $node_primary3->stop;
$node_standby3->stop;
# =============================================================================
-# Testcase start: Check last_inactive_time property of streaming standby's slot
+# Testcase start: Check last_inactive_time property of the streaming standby's
slot
#
# Initialize primary node
@@ -440,8 +440,8 @@ $primary4->safe_psql(
SELECT pg_create_physical_replication_slot(slot_name := '$sb4_slot');
]);
-# Get last_inactive_time value after slot's creation. Note that the slot is
still
-# inactive unless it's used by the standby below.
+# Get last_inactive_time value after the slot's creation. Note that the slot
+# is still inactive till it's used by the standby below.
my $last_inactive_time = $primary4->safe_psql('postgres',
qq(SELECT last_inactive_time FROM pg_replication_slots WHERE slot_name
= '$sb4_slot' AND last_inactive_time IS NOT NULL;)
);
@@ -470,8 +470,8 @@ is( $primary4->safe_psql(
# Stop the standby to check its last_inactive_time value is updated
$standby4->stop;
-# Let's also restart the primary so that the last_inactive_time is set upon
-# loading the slot from disk.
+# Let's restart the primary so that the last_inactive_time is set upon
+# loading the slot from the disk.
$primary4->restart;
is( $primary4->safe_psql(
@@ -483,11 +483,11 @@ is( $primary4->safe_psql(
$standby4->stop;
-# Testcase end: Check last_inactive_time property of streaming standby's slot
+# Testcase end: Check last_inactive_time property of the streaming standby's
slot
# =============================================================================
# =============================================================================
-# Testcase start: Check last_inactive_time property of logical subscriber's
slot
+# Testcase start: Check last_inactive_time property of the logical
subscriber's slot
my $publisher4 = $primary4;
# Create subscriber node
@@ -508,8 +508,8 @@ $publisher4->safe_psql('postgres',
"SELECT pg_create_logical_replication_slot(slot_name := '$lsub4_slot',
plugin := 'pgoutput');"
);
-# Get last_inactive_time value after slot's creation. Note that the slot is
still
-# inactive unless it's used by the subscriber below.
+# Get last_inactive_time value after the slot's creation. Note that the slot
+# is still inactive till it's used by the subscriber below.
$last_inactive_time = $publisher4->safe_psql('postgres',
qq(SELECT last_inactive_time FROM pg_replication_slots WHERE slot_name
= '$lsub4_slot' AND last_inactive_time IS NOT NULL;)
);
@@ -541,8 +541,8 @@ is( $publisher4->safe_psql(
# Stop the subscriber to check its last_inactive_time value is updated
$subscriber4->stop;
-# Let's also restart the publisher so that the last_inactive_time is set upon
-# loading the slot from disk.
+# Let's restart the publisher so that the last_inactive_time is set upon
+# loading the slot from the disk.
$publisher4->restart;
is( $publisher4->safe_psql(
@@ -552,7 +552,7 @@ is( $publisher4->safe_psql(
't',
'last inactive time for an inactive logical slot is updated correctly');
-# Testcase end: Check last_inactive_time property of logical subscriber's slot
+# Testcase end: Check last_inactive_time property of the logical subscriber's
slot
# =============================================================================
$publisher4->stop;
