On Sun, Sep 7, 2025 at 1:32 PM Fabrice Chapuis <fabrice636861@gmail.com> wrote: > > Hi, > Here the version 1 of the patch with the modifications. > I do a git diff --check until there are no more errors. > In a next version of the patch, I think I have make change to call ReplicationSlotCreate() function with the value of the flag allow_overwrite be consistent
In which flow? I see all ReplicationSlotCreate() references already accepting the value.
> and modify the interface ReplicationSlotCreate to display the attribute allow_overwrite. >
Do you mean to modify pg_replication_slots?
Thank You for the patch. Please find a few comments:
1) In both create_physical_replication_slot() and create_physical_replication_slot(): + false, false,false);
,false --> , false (space after comma is recommended)
2) + elog(DEBUG1, "logical replication slot %s created with option allow_overwrite to %s", + NameStr(slot->data.name), slot->data.allow_overwrite ? "true" : "false");
IMO, we don't need this as we do not log other properties of the slot as well.
3) We can have pg_replication_slots (pg_get_replication_slots) modified to display the new property. Also we can modify docs to have the new property defined.
4) + { + /* Check if we need to overwrite an existing logical slot */ + if (allow_overwrite) + { + /* Get rid of a replication slot that is no longer wanted */ + ReplicationSlotDrop(remote_slot->name,true); + + /* Get rid of a replication slot that is no longer wanted */ + ereport(WARNING, + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("slot \"%s\" already exists" + " on the standby but it will be dropped because " + "flag allow_overwrite is set to true", + remote_slot->name)); + + /* Going back to the main loop after droping the failover slot */ + return false;
Currently we are returning after dropping the slot. But I think we shall drop the slot and proceed with new slot creation of the same name otherwise we may be left with old slot dropped and no new slot created specially when API is run.
Scenario: --On primary, create 2 slots: slot1 and slot2. --On standby, create one slot slot1 with allow_overwrite=true. --Run 'SELECT pg_sync_replication_slots();' on standby.
At the end of API, expectation is that both slots will be present with 'synced'=true, but only slot2 is present. if we run this API next time, slot1 is created. It should have been dropped and recreated (or say overwritten) in the first run itself.
5) IMO, LOG is sufficient here, as the action aligns with the user-provided 'allow_overwrite' setting.
