Re: Issue with logical replication slot during switchover

On Fri, 29 Aug 2025 at 19:28, Fabrice Chapuis <fabrice636861(at)gmail(dot)com> wrote:
>
> Thanks Shveta for your procedure to follow, here is the version 1 of the patch
>
> Regards,
>
> Fabrice
>
> On Fri, Aug 29, 2025 at 5:34 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>>
>> On Thu, Aug 28, 2025 at 9:11 PM Fabrice Chapuis <fabrice636861(at)gmail(dot)com> wrote:
>> >
>> > What is the procedure to create this patch. Thank you for your help.
>> >
>>
>> We use 'git format-patch' to generate formatted patches. I have given
>> a few links ([1],[2],[3)] on know-how.
>>
>> I usually use these steps:
>>
>>
>> git add <file1.c> <adds file to staging area>
>> git add <file2.c>
>> <once all the required files are added, create a commit>
>> git commit <add the required message to describe the patch>
>> git format-patch -1 -v1 <create the patch>
>> -1-> to create patch for last 1 commit
>> -v1 --> patch-prefix. I have used v1 which indicates version-1, when
>> we upload the patch a second time, we can use -v2 and so on.
>>
>> [1]: https://stackoverflow.com/questions/6658313/how-can-i-generate-a-git-patch-for-a-specific-commit
>> [2]: https://www.geeksforgeeks.org/git/how-to-generate-a-git-patch-for-a-specific-commit/
>> [3]: https://gist.github.com/ganapathichidambaram/3504e31fc8ba809762b3a0d9d9ef8ec2
>>
>> thanks
>> Shveta

Hi Fabrice,

Thanks for providing the patch. I reviewed your patch and have
following comment:

1. I think we should add a commit message in the patch. It would help
to give an understanding of the patch.

2. I tried applying patch on HEAD, it generates following warnings:
Applying: fix failover slot issue when doing a switchover
.git/rebase-apply/patch:31: trailing whitespace.
bool allow_overwrite = false;
.git/rebase-apply/patch:45: trailing whitespace.
bool synced;
.git/rebase-apply/patch:55: trailing whitespace.

.git/rebase-apply/patch:56: trailing whitespace.
if (!synced){
.git/rebase-apply/patch:57: trailing whitespace.
/*
warning: squelched 16 whitespace errors
warning: 21 lines add whitespace errors.

3. I also tried to build the patch on HEAD and it is giving the following error:
ubuntu(at)ubuntu-Virtual-Machine:~/Project/pg/postgres$
./../../install-clean.sh pg_30_8 > warn.log
launcher.c: In function ‘CreateConflictDetectionSlot’:
launcher.c:1457:9: error: too few arguments to function ‘ReplicationSlotCreate’
1457 | ReplicationSlotCreate(CONFLICT_DETECTION_SLOT, false,
RS_PERSISTENT, false,
| ^~~~~~~~~~~~~~~~~~~~~
In file included from launcher.c:35:
../../../../src/include/replication/slot.h:307:13: note: declared here
307 | extern void ReplicationSlotCreate(const char *name, bool db_specific,
| ^~~~~~~~~~~~~~~~~~~~~
make[4]: *** [<builtin>: launcher.o] Error 1
make[3]: *** [../../../src/backend/common.mk:37: logical-recursive] Error 2
make[2]: *** [common.mk:37: replication-recursive] Error 2
make[1]: *** [Makefile:42: install-backend-recurse] Error 2
make: *** [GNUmakefile:11: install-src-recurse] Error 2

4. I have some general comments regarding formatting of the patch.
+ // Both local and remote slot have the same name

We use following format for single line comments:
/* comment text */
and for multi line comments we use following format:
/*
* comment text begins here
* and continues here
*/

5. We should use proper indentation here:
+ elog(LOG, "Logical replication slot %s created with option
allow_overwrite to %s",
+ NameStr(slot->data.name),
+ slot->data.allow_overwrite ? "true" : "false");

6.
+ if (!synced){
+ /*
+ * Check if we need to overwrite an existing
+ * logical slot
+ */
We should start the parentheses from the next line.
Also, indentation for comment is not correct here.

7.
+ if (allow_overwrite){
+ /*
+ * Get rid of a replication slot that is no
+ *longer wanted
+ */
Similar comment as above.

Please refer [1] [2] for proper formatting of the patch.
[1]: https://www.postgresql.org/docs/devel/source-format.html
[2]: https://wiki.postgresql.org/wiki/Creating_Clean_Patches

Thanks,
Shlok Kyal