public inbox for [email protected]
help / color / mirror / Atom feedFrom: Bharath Rupireddy <[email protected]>
To: Bertrand Drouvot <[email protected]>
Cc: shveta malik <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Nathan Bossart <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Introduce XID age and inactive timeout based replication slot invalidation
Date: Wed, 3 Apr 2024 17:12:12 +0530
Message-ID: <CALj2ACUmp-F+irHYfNhmo_9yaCo7aGi7ZB20KQcDR3S9eT56hg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAJpy0uDgKwk=hOE-Q5z4nA5kovHKhuspjXn2MhSmApbQW2ck9Q@mail.gmail.com>
<CALj2ACVPaiTHokRzV135UycNozKsLwa-jvcjX7G7rFbHYJLjaA@mail.gmail.com>
<[email protected]>
<CALj2ACX1bdaX2gf5ttwk3buiDrgZVV=vM6S1SQKf=Sxhj1HkzQ@mail.gmail.com>
<CAA4eK1JU14-Fz3Ttx_wy=vzDaeTSv=umt7sC3N+4k2aYcdk4=w@mail.gmail.com>
<CALj2ACWOz0tEY2vi3OneittGj9+yrQ9A6+6YYb1Cj7L_QrdMVg@mail.gmail.com>
<CAD21AoAd0BX0L17YxSCbSNbj=3V7dvLVGY0WhwLHAt724rNCZA@mail.gmail.com>
<[email protected]>
<CAJpy0uBthb-RG=Xg+mM-G6Xy01JLcBsizJuz+xbti08qP_W7AQ@mail.gmail.com>
<CALj2ACVfDBcbybgyga7ia_KiQPAfEi_3=VDOGf2VU8RkJZSQpw@mail.gmail.com>
<[email protected]>
On Wed, Apr 3, 2024 at 12:20 PM Bertrand Drouvot
<[email protected]> wrote:
>
> > Please find the attached v31 patches implementing the above idea:
>
> Some comments related to v31-0001:
>
> === testing the behavior
>
> T1 ===
>
> > - synced slots get their on inactive_since just like any other slot
>
> It behaves as described.
>
> T2 ===
>
> > - synced slots inactive_since is set to current timestamp after the
> > standby gets promoted to help inactive_since interpret correctly just
> > like any other slot.
>
> It behaves as described.
Thanks for testing.
> CR1 ===
>
> + <structfield>inactive_since</structfield> value will get updated
> + after every synchronization
>
> indicates the last synchronization time? (I think that after every synchronization
> could lead to confusion).
Done.
> CR2 ===
>
> + /*
> + * Set the time since the slot has become inactive after shutting
> + * down slot sync machinery. This helps correctly interpret the
> + * time if the standby gets promoted without a restart.
> + */
>
> It looks to me that this comment is not at the right place because there is
> nothing after the comment that indicates that we shutdown the "slot sync machinery".
>
> Maybe a better place is before the function definition and mention that this is
> currently called when we shutdown the "slot sync machinery"?
Done.
> CR3 ===
>
> + * We get the current time beforehand and only once to avoid
> + * system calls overhead while holding the lock.
>
> s/avoid system calls overhead while holding the lock/avoid system calls while holding the spinlock/?
Done.
> CR4 ===
>
> + * Set the time since the slot has become inactive. We get the current
> + * time beforehand to avoid system call overhead while holding the lock
>
> Same.
Done.
> CR5 ===
>
> + # Check that the captured time is sane
> + if (defined $reference_time)
> + {
>
> s/Check that the captured time is sane/Check that the inactive_since is sane/?
>
> Sorry if some of those comments could have been done while I did review v29-0001.
Done.
On Wed, Apr 3, 2024 at 2:58 PM shveta malik <[email protected]> wrote:
>
> Thanks for the patches, please find few comments:
>
> v31-001:
>
> 1)
> system-views.sgml:
> value will get updated after every synchronization from the
> corresponding remote slot on the primary.
>
> --This is confusing. It will be good to rephrase it.
Done as per Bertrand's suggestion.
> 2)
> update_synced_slots_inactive_since()
>
> --May be, we should mention in the header that this function is called
> only during promotion.
Done as per Bertrand's suggestion.
> 3) 040_standby_failover_slots_sync.pl:
> We capture inactive_since_on_primary when we do this for the first time at #175
> ALTER SUBSCRIPTION regress_mysub1 DISABLE"
>
> But we again recreate the sub and disable it at line #280.
> Do you think we shall get inactive_since_on_primary again here, to be
> compared with inactive_since_on_new_primary later?
Hm. Done that. Recapturing both slot_creation_time_on_primary and
inactive_since_on_primary before and after CREATE SUBSCRIPTION creates
the slot again on the primary/publisher.
On Wed, Apr 3, 2024 at 3:32 PM Amit Kapila <[email protected]> wrote:
>
> > CR2 ===
> >
> > + /*
> > + * Set the time since the slot has become inactive after shutting
> > + * down slot sync machinery. This helps correctly interpret the
> > + * time if the standby gets promoted without a restart.
> > + */
> >
> > It looks to me that this comment is not at the right place because there is
> > nothing after the comment that indicates that we shutdown the "slot sync machinery".
> >
> > Maybe a better place is before the function definition and mention that this is
> > currently called when we shutdown the "slot sync machinery"?
> >
> Won't it be better to have an assert for SlotSyncCtx->pid? IIRC, we
> have some existing issues where we don't ensure that no one is running
> sync API before shutdown is complete but I think we can deal with that
> separately and here we can still have an Assert.
That can work to ensure the slot sync worker isn't running as
SlotSyncCtx->pid gets updated only for the slot sync worker. I added
this assertion for now.
We need to ensure (in a separate patch and thread) there is no backend
acquiring it and performing sync while the slot sync worker is
shutting down. Otherwise, some of the slots can get resynced and some
are not while we are shutting down the slot sync worker as part of the
standby promotion which might leave the slots in an inconsistent
state.
> > CR3 ===
> >
> > + * We get the current time beforehand and only once to avoid
> > + * system calls overhead while holding the lock.
> >
> > s/avoid system calls overhead while holding the lock/avoid system calls while holding the spinlock/?
> >
> Is it valid to say that there is overhead of this call while holding
> spinlock? Because I don't think at the time of promotion we expect any
> other concurrent slot activity. The first reason seems good enough.
No slot activity but why GetCurrentTimestamp needs to be called every
time in a loop.
> One other observation:
> --- a/src/backend/replication/slot.c
> +++ b/src/backend/replication/slot.c
> @@ -42,6 +42,7 @@
> #include "access/transam.h"
> #include "access/xlog_internal.h"
> #include "access/xlogrecovery.h"
> +#include "access/xlogutils.h"
>
> Is there a reason for this inclusion? I don't see any change which
> should need this one.
Not anymore. It was earlier needed for using the InRecovery flag in
the then approach.
On Wed, Apr 3, 2024 at 4:19 PM Amit Kapila <[email protected]> wrote:
>
> > 3) 040_standby_failover_slots_sync.pl:
> > We capture inactive_since_on_primary when we do this for the first time at #175
> > ALTER SUBSCRIPTION regress_mysub1 DISABLE"
> >
> > But we again recreate the sub and disable it at line #280.
> > Do you think we shall get inactive_since_on_primary again here, to be
> > compared with inactive_since_on_new_primary later?
> >
>
> I think so.
Modified this to recapture the times before and after the slot gets recreated.
> Few additional comments on tests:
> 1.
> +is( $standby1->safe_psql(
> + 'postgres',
> + "SELECT '$inactive_since_on_primary'::timestamptz <
> '$inactive_since_on_standby'::timestamptz AND
> + '$inactive_since_on_standby'::timestamptz < '$slot_sync_time'::timestamptz;"
>
> Shall we do <= check as we are doing in the main function
> get_slot_inactive_since_value as the time duration is less so it can
> be the same as well? Similarly, please check other tests.
I get you. If the tests are so fast that losing a bit of precision
might cause tests to fail. So, I'v added equality check for all the
tests.
> 2.
> +=item $node->get_slot_inactive_since_value(self, slot_name, reference_time)
> +
> +Get inactive_since column value for a given replication slot validating it
> +against optional reference time.
> +
> +=cut
> +
> +sub get_slot_inactive_since_value
>
> I see that all callers validate against reference time. It is better
> to name it validate_slot_inactive_since rather than using get_* as the
> main purpose is to validate the passed value.
Existing callers yes. Also, I've removed the reference time as an
optional parameter.
Per an offlist chat with Amit, I've added the following note in
synchronize_one_slot:
@@ -584,6 +585,11 @@ synchronize_one_slot(RemoteSlot *remote_slot, Oid
remote_dbid)
* overwriting 'invalidated' flag to remote_slot's value. See
* InvalidatePossiblyObsoleteSlot() where it invalidates slot directly
* if the slot is not acquired by other processes.
+ *
+ * XXX: If it ever turns out that slot acquire/release is costly for
+ * cases when none of the slot property is changed then we can do a
+ * pre-check to ensure that at least one of the slot property is
+ * changed before acquiring the slot.
*/
ReplicationSlotAcquire(remote_slot->name, true);
Please find the attached v32-0001 patch with the above review comments
addressed. I'm working on review comments for 0002.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[application/octet-stream] v32-0001-Allow-synced-slots-to-have-their-own-inactive_si.patch (16.0K, ../CALj2ACUmp-F+irHYfNhmo_9yaCo7aGi7ZB20KQcDR3S9eT56hg@mail.gmail.com/2-v32-0001-Allow-synced-slots-to-have-their-own-inactive_si.patch)
download | inline diff:
From 63230aab91d5447a384a5c9d1723675f3b0ac4de Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Wed, 3 Apr 2024 11:40:11 +0000
Subject: [PATCH v32] Allow synced slots to have their own inactive_since.
The slot's inactive_since isn't currently maintained for
synced slots on the standby. The commit a11f330b55 prevents
updating inactive_since with RecoveryInProgress() check in
RestoreSlotFromDisk(). But, the issue is that
RecoveryInProgress() always returns true in
RestoreSlotFromDisk() as 'xlogctl->SharedRecoveryState' is
always 'RECOVERY_STATE_CRASH' at that time. Because of this,
inactive_since is always NULL on a promoted standby for all
synced slots even after server restart.
Above issue led us to a question as to why we can't just let
standby maintain its own inactive_since for synced slots. This is
consistent with any regular slots and also indicates the last
synchronization time of the slot. This approach simplifies things
when compared to just copying inactive_since received from the
remote slot on the primary (for instance, there can exists clock
drift between primary and standby so just copying inactive_since
from the primary slot to the standby sync slot may not represent
the correct value).
This commit does two things:
1) Maintains inactive_since for sync slots whenever the slot is
released just like any other regular slot.
2) Ensures the value is set to current timestamp during the
shutdown of slot sync machinery to help correctly interpret the
time if the standby gets promoted without a restart.
Author: Bharath Rupireddy
Reviewed-by: Bertrand Drouvot, Amit Kapila, Shveta Malik
Discussion: https://www.postgresql.org/message-id/CALj2ACWLctoiH-pSjWnEpR54q4DED6rw_BRJm5pCx86_Y01MoQ%40mail.gmail.com
---
doc/src/sgml/system-views.sgml | 7 +++
src/backend/replication/logical/slotsync.c | 51 +++++++++++++++
src/backend/replication/slot.c | 22 +++----
src/test/perl/PostgreSQL/Test/Cluster.pm | 31 ++++++++++
src/test/recovery/t/019_replslot_limit.pl | 30 ++-------
.../t/040_standby_failover_slots_sync.pl | 62 +++++++++++++++++++
6 files changed, 162 insertions(+), 41 deletions(-)
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 3c8dca8ca3..7ed617170f 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -2530,6 +2530,13 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
<para>
The time since the slot has become inactive.
<literal>NULL</literal> if the slot is currently being used.
+ Note that for slots on the standby that are being synced from a
+ primary server (whose <structfield>synced</structfield> field is
+ <literal>true</literal>), the
+ <structfield>inactive_since</structfield> indicates the last
+ synchronization (see
+ <xref linkend="logicaldecoding-replication-slots-synchronization"/>)
+ time.
</para></entry>
</row>
diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c
index 9ac847b780..755bf40a9a 100644
--- a/src/backend/replication/logical/slotsync.c
+++ b/src/backend/replication/logical/slotsync.c
@@ -150,6 +150,7 @@ typedef struct RemoteSlot
} RemoteSlot;
static void slotsync_failure_callback(int code, Datum arg);
+static void update_synced_slots_inactive_since(void);
/*
* If necessary, update the local synced slot's metadata based on the data
@@ -584,6 +585,11 @@ synchronize_one_slot(RemoteSlot *remote_slot, Oid remote_dbid)
* overwriting 'invalidated' flag to remote_slot's value. See
* InvalidatePossiblyObsoleteSlot() where it invalidates slot directly
* if the slot is not acquired by other processes.
+ *
+ * XXX: If it ever turns out that slot acquire/release is costly for
+ * cases when none of the slot property is changed then we can do a
+ * pre-check to ensure that at least one of the slot property is
+ * changed before acquiring the slot.
*/
ReplicationSlotAcquire(remote_slot->name, true);
@@ -1355,6 +1361,48 @@ ReplSlotSyncWorkerMain(char *startup_data, size_t startup_data_len)
Assert(false);
}
+/*
+ * Update the inactive_since property for synced slots.
+ *
+ * Note that this function is currently called when we shutdown the slot sync
+ * machinery. This helps correctly interpret the inactive_since if the standby
+ * gets promoted without a restart.
+ */
+static void
+update_synced_slots_inactive_since(void)
+{
+ TimestampTz now = 0;
+
+ /* The slot sync worker mustn't be running by now */
+ Assert(SlotSyncCtx->pid == InvalidPid);
+
+ LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);
+
+ for (int i = 0; i < max_replication_slots; i++)
+ {
+ ReplicationSlot *s = &ReplicationSlotCtl->replication_slots[i];
+
+ /* Check if it is a synchronized slot */
+ if (s->in_use && s->data.synced)
+ {
+ Assert(SlotIsLogical(s));
+
+ /*
+ * We get the current time beforehand and only once to avoid
+ * system calls while holding the spinlock.
+ */
+ if (now == 0)
+ now = GetCurrentTimestamp();
+
+ SpinLockAcquire(&s->mutex);
+ s->inactive_since = now;
+ SpinLockRelease(&s->mutex);
+ }
+ }
+
+ LWLockRelease(ReplicationSlotControlLock);
+}
+
/*
* Shut down the slot sync worker.
*/
@@ -1368,6 +1416,7 @@ ShutDownSlotSync(void)
if (SlotSyncCtx->pid == InvalidPid)
{
SpinLockRelease(&SlotSyncCtx->mutex);
+ update_synced_slots_inactive_since();
return;
}
SpinLockRelease(&SlotSyncCtx->mutex);
@@ -1400,6 +1449,8 @@ ShutDownSlotSync(void)
}
SpinLockRelease(&SlotSyncCtx->mutex);
+
+ update_synced_slots_inactive_since();
}
/*
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index d778c0b921..3bddaae022 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -690,13 +690,10 @@ ReplicationSlotRelease(void)
}
/*
- * Set the last inactive time after marking the slot inactive. We don't
- * set it for the slots currently being synced from the primary to the
- * standby because such slots are typically inactive as decoding is not
- * allowed on those.
+ * Set the time since the slot has become inactive. We get the current
+ * time beforehand to avoid system call while holding the spinlock.
*/
- if (!(RecoveryInProgress() && slot->data.synced))
- now = GetCurrentTimestamp();
+ now = GetCurrentTimestamp();
if (slot->data.persistency == RS_PERSISTENT)
{
@@ -2369,16 +2366,11 @@ RestoreSlotFromDisk(const char *name)
slot->active_pid = 0;
/*
- * 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. We don't set it for the slots currently being synced
- * from the primary to the standby because such slots are typically
- * inactive as decoding is not allowed on those.
+ * Set the time since the slot has become inactive after loading the
+ * slot from the disk into memory. Whoever acquires the slot i.e.
+ * makes the slot active will reset it.
*/
- if (!(RecoveryInProgress() && slot->data.synced))
- slot->inactive_since = GetCurrentTimestamp();
- else
- slot->inactive_since = 0;
+ slot->inactive_since = GetCurrentTimestamp();
restored = true;
break;
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index b08296605c..d68db6a6f9 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -3276,6 +3276,37 @@ sub create_logical_slot_on_standby
=pod
+=item $node->validate_slot_inactive_since(self, slot_name, reference_time)
+
+Validate inactive_since value of a given replication slot against the reference
+time and return it.
+
+=cut
+
+sub validate_slot_inactive_since
+{
+ my ($self, $slot_name, $reference_time) = @_;
+ my $name = $self->name;
+
+ my $inactive_since = $self->safe_psql('postgres',
+ qq(SELECT inactive_since FROM pg_replication_slots
+ WHERE slot_name = '$slot_name' AND inactive_since IS NOT NULL;)
+ );
+
+ # Check that the inactive_since is sane
+ is($self->safe_psql('postgres',
+ qq[SELECT '$inactive_since'::timestamptz > to_timestamp(0) AND
+ '$inactive_since'::timestamptz >= '$reference_time'::timestamptz;]
+ ),
+ 't',
+ "last inactive time for slot $slot_name is valid on node $name")
+ or die "could not validate captured inactive_since for slot $slot_name";
+
+ return $inactive_since;
+}
+
+=pod
+
=item $node->advance_wal(num)
Advance WAL of node by given number of segments.
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index 3b9a306a8b..712141a33b 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -443,7 +443,7 @@ $primary4->safe_psql(
# Get inactive_since value after the slot's creation. Note that the slot is
# still inactive till it's used by the standby below.
my $inactive_since =
- capture_and_validate_slot_inactive_since($primary4, $sb4_slot, $slot_creation_time);
+ $primary4->validate_slot_inactive_since($sb4_slot, $slot_creation_time);
$standby4->start;
@@ -467,7 +467,7 @@ $primary4->restart;
is( $primary4->safe_psql(
'postgres',
- qq[SELECT inactive_since > '$inactive_since'::timestamptz FROM pg_replication_slots WHERE slot_name = '$sb4_slot' AND inactive_since IS NOT NULL;]
+ qq[SELECT inactive_since >= '$inactive_since'::timestamptz FROM pg_replication_slots WHERE slot_name = '$sb4_slot' AND inactive_since IS NOT NULL;]
),
't',
'last inactive time for an inactive physical slot is updated correctly');
@@ -502,7 +502,7 @@ $publisher4->safe_psql('postgres',
# Get inactive_since value after the slot's creation. Note that the slot is
# still inactive till it's used by the subscriber below.
$inactive_since =
- capture_and_validate_slot_inactive_since($publisher4, $lsub4_slot, $slot_creation_time);
+ $publisher4->validate_slot_inactive_since($lsub4_slot, $slot_creation_time);
$subscriber4->start;
$subscriber4->safe_psql('postgres',
@@ -529,7 +529,7 @@ $publisher4->restart;
is( $publisher4->safe_psql(
'postgres',
- qq[SELECT inactive_since > '$inactive_since'::timestamptz FROM pg_replication_slots WHERE slot_name = '$lsub4_slot' AND inactive_since IS NOT NULL;]
+ qq[SELECT inactive_since >= '$inactive_since'::timestamptz FROM pg_replication_slots WHERE slot_name = '$lsub4_slot' AND inactive_since IS NOT NULL;]
),
't',
'last inactive time for an inactive logical slot is updated correctly');
@@ -540,26 +540,4 @@ is( $publisher4->safe_psql(
$publisher4->stop;
$subscriber4->stop;
-# Capture and validate inactive_since of a given slot.
-sub capture_and_validate_slot_inactive_since
-{
- my ($node, $slot_name, $slot_creation_time) = @_;
-
- my $inactive_since = $node->safe_psql('postgres',
- qq(SELECT inactive_since FROM pg_replication_slots
- WHERE slot_name = '$slot_name' AND inactive_since IS NOT NULL;)
- );
-
- # Check that the captured time is sane
- is( $node->safe_psql(
- 'postgres',
- qq[SELECT '$inactive_since'::timestamptz > to_timestamp(0) AND
- '$inactive_since'::timestamptz >= '$slot_creation_time'::timestamptz;]
- ),
- 't',
- "last inactive time for an active slot $slot_name is sane");
-
- return $inactive_since;
-}
-
done_testing();
diff --git a/src/test/recovery/t/040_standby_failover_slots_sync.pl b/src/test/recovery/t/040_standby_failover_slots_sync.pl
index 869e3d2e91..a8be8ac7fc 100644
--- a/src/test/recovery/t/040_standby_failover_slots_sync.pl
+++ b/src/test/recovery/t/040_standby_failover_slots_sync.pl
@@ -35,6 +35,13 @@ my $subscriber1 = PostgreSQL::Test::Cluster->new('subscriber1');
$subscriber1->init;
$subscriber1->start;
+# Capture the time before the logical failover slot is created on the
+# primary. We later call this publisher as primary anyway.
+my $slot_creation_time_on_primary = $publisher->safe_psql(
+ 'postgres', qq[
+ SELECT current_timestamp;
+]);
+
# Create a slot on the publisher with failover disabled
$publisher->safe_psql('postgres',
"SELECT 'init' FROM pg_create_logical_replication_slot('lsub1_slot', 'pgoutput', false, false, false);"
@@ -174,6 +181,11 @@ $primary->poll_query_until(
"SELECT COUNT(*) FROM pg_catalog.pg_replication_slots WHERE slot_name = 'lsub1_slot' AND active = 'f'",
1);
+# Capture the inactive_since of the slot from the primary. Note that the slot
+# is not yet active.
+my $inactive_since_on_primary =
+ $primary->validate_slot_inactive_since('lsub1_slot', $slot_creation_time_on_primary);
+
# Wait for the standby to catch up so that the standby is not lagging behind
# the subscriber.
$primary->wait_for_replay_catchup($standby1);
@@ -181,6 +193,11 @@ $primary->wait_for_replay_catchup($standby1);
# Synchronize the primary server slots to the standby.
$standby1->safe_psql('postgres', "SELECT pg_sync_replication_slots();");
+my $slot_sync_time = $standby1->safe_psql(
+ 'postgres', qq[
+ SELECT current_timestamp;
+]);
+
# Confirm that the logical failover slots are created on the standby and are
# flagged as 'synced'
is( $standby1->safe_psql(
@@ -190,6 +207,19 @@ is( $standby1->safe_psql(
"t",
'logical slots have synced as true on standby');
+# Capture the inactive_since of the synced slot on the standby
+my $inactive_since_on_standby =
+ $standby1->validate_slot_inactive_since('lsub1_slot', $slot_creation_time_on_primary);
+
+# Synced slot on the standby must get its own inactive_since.
+is( $standby1->safe_psql(
+ 'postgres',
+ "SELECT '$inactive_since_on_primary'::timestamptz <= '$inactive_since_on_standby'::timestamptz AND
+ '$inactive_since_on_standby'::timestamptz <= '$slot_sync_time'::timestamptz;"
+ ),
+ "t",
+ 'synchronized slot has got its own inactive_since');
+
##################################################
# Test that the synchronized slot will be dropped if the corresponding remote
# slot on the primary server has been dropped.
@@ -237,6 +267,13 @@ is( $standby1->safe_psql(
$standby1->append_conf('postgresql.conf', 'max_slot_wal_keep_size = -1');
$standby1->reload;
+# Capture the time before the logical failover slot is created on the primary.
+# Note that the subscription creates the slot again on the primary.
+$slot_creation_time_on_primary = $publisher->safe_psql(
+ 'postgres', qq[
+ SELECT current_timestamp;
+]);
+
# To ensure that restart_lsn has moved to a recent WAL position, we re-create
# the subscription and the logical slot.
$subscriber1->safe_psql(
@@ -257,6 +294,11 @@ $primary->poll_query_until(
"SELECT COUNT(*) FROM pg_catalog.pg_replication_slots WHERE slot_name = 'lsub1_slot' AND active = 'f'",
1);
+# Capture the inactive_since of the slot from the primary. Note that the slot
+# is not yet active but has been dropped and recreated.
+$inactive_since_on_primary =
+ $primary->validate_slot_inactive_since('lsub1_slot', $slot_creation_time_on_primary);
+
# Wait for the standby to catch up so that the standby is not lagging behind
# the subscriber.
$primary->wait_for_replay_catchup($standby1);
@@ -808,8 +850,28 @@ $primary->reload;
$standby1->start;
$primary->wait_for_replay_catchup($standby1);
+# Capture the time before the standby is promoted
+my $promotion_time_on_primary = $standby1->safe_psql(
+ 'postgres', qq[
+ SELECT current_timestamp;
+]);
+
$standby1->promote;
+# Capture the inactive_since of the synced slot after the promotion.
+# Expectation here is that the slot gets its own inactive_since as part of the
+# promotion. We do this check before the slot is enabled on the new primary
+# below, otherwise the slot gets active setting inactive_since to NULL.
+my $inactive_since_on_new_primary =
+ $standby1->validate_slot_inactive_since('lsub1_slot', $promotion_time_on_primary);
+
+is( $standby1->safe_psql(
+ 'postgres',
+ "SELECT '$inactive_since_on_new_primary'::timestamptz >= '$inactive_since_on_primary'::timestamptz"
+ ),
+ "t",
+ 'synchronized slot has got its own inactive_since on the new primary after promotion');
+
# Update subscription with the new primary's connection info
my $standby1_conninfo = $standby1->connstr . ' dbname=postgres';
$subscriber1->safe_psql('postgres',
--
2.34.1
view thread (3+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Introduce XID age and inactive timeout based replication slot invalidation
In-Reply-To: <CALj2ACUmp-F+irHYfNhmo_9yaCo7aGi7ZB20KQcDR3S9eT56hg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox