On Tue, Sep 23, 2025 at 1:11 PM Ajin Cherian <itsa...@gmail.com> wrote:
>
> Attaching v3 with the above code changes.
>
Thanks for the patch. Please find a few comments:
1)
+ if (!StandbyMode && slot->data.synced)
+ {
+ ReplicationSlotAcquire(NameStr(slot->data.name), false, true);
+ slot->data.synced = false;
+ ReplicationSlotMarkDirty();
+ ReplicationSlotRelease();
+ }
Do we really need to acquire in this startup flow? Can we directly
set the 'just_dirtied' and 'dirty' as true without calling
ReplicationSlotMarkDirty(). IMO, there is no possibility of another
session connection at this point right as we have not started up
completely, and thus it should be okay to directly mark it as dirty
without acquiring slot or taking locks.
2)
+ * If this was a promotion, first reset any slots that had been marked as
+ * synced during standby mode. Although slots that are marked as synced
+ * are reset on a restart of the primary, we need to do it in the promotion
+ * path as it could be some time before the next restart.
Shall we rephrase to:
If this was a promotion, first reset the synced flag for any logical
slots if it's set. Although the synced flag for logical slots is reset
on every primary restart, we also need to handle it during promotion
since existing backend sessions remain active even after promotion,
and a restart may not happen for some time.
3)
+ ereport(LOG,
+ (errmsg("reset synced flag for replication slot \"%s\"",
+ NameStr(s->data.name))));
a) Shall we change it to DEBUG1?
b) Shall the msg be:
synced flag reset for replication slot \"%s\" during promotion
4)
Regarding:
-# Confirm the synced slot 'lsub1_slot' is retained on the new primary
+# Confirm the synced slot 'lsub1_slot' is reset on the new primary
is( $standby1->safe_psql(
'postgres',
q{SELECT count(*) = 2 FROM pg_replication_slots WHERE slot_name IN
('lsub1_slot', 'snap_test_slot') AND synced AND NOT temporary;}
),
- 't',
- 'synced slot retained on the new primary');
+ 'f',
+ 'synced slot reset on the new primary');
I think the original test was trying to confirm that both the logical
slots are retained after promotion. See test-title:
# Promote the standby1 to primary. Confirm that:
# a) the slot 'lsub1_slot' and 'snap_test_slot' are retained on the new primary
But with this change, it will not be able to verify that. Shall we
modify the test to say 'Confirm that the slots 'lsub1_slot' and
'snap_test_slot' are retained on the new primary and synced flag is
cleared' and change the query to have 'NOT synced'. Thoughts?
thanks
Shveta
