Re: Issue with logical replication slot during switchover - Mailing list pgsql-hackers

On Fri, 29 Aug 2025 at 19:28, Fabrice Chapuis <fabrice636861@gmail.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.malik@gmail.com> wrote:
>>
>> On Thu, Aug 28, 2025 at 9:11 PM Fabrice Chapuis <fabrice636861@gmail.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@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