Re: [PoC] pg_upgrade: allow to upgrade publisher node - Mailing list pgsql-hackers

On Wed, 8 Nov 2023 at 08:43, vignesh C <vignesh21@gmail.com> wrote:
>
> On Tue, 7 Nov 2023 at 13:25, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Tue, Nov 7, 2023 at 10:01 AM Zhijie Hou (Fujitsu)
> > <houzj.fnst@fujitsu.com> wrote:
> > >
> > > On Tuesday, November 7, 2023 12:14 PM Kuroda, Hayato/黒田 隼人 <kuroda.hayato@fujitsu.com> wrote:
> > > >
> > > > Dear hackers,
> > > >
> > > > PSA the patch to solve the issue [1].
> > > >
> > > > Kindly Peter E. and Andrew raised an issue that delete_old_cluster.sh is
> > > > generated in the source directory, even when the VPATH/meson build.
> > > > This can avoid by changing the directory explicitly.
> > > >
> > > > [1]:
> > > > https://www.postgresql.org/message-id/flat/7b8a9460-5668-b372-04e6-7b
> > > > 52e9308493%40dunslane.net#554090099bbbd12c94bf570665a6badf
> > >
> > > Thanks for the patch, I have confirmed that the files won't be generated
> > > in source directory after applying the patch.
> > >
> > > After running: "meson test -C build/ --suite pg_upgrade",
> > > The files are in the test directory:
> > > ./build/testrun/pg_upgrade/003_logical_slots/data/delete_old_cluster.sh
> > >
> >
> > Thanks for the patch and verification. Pushed the fix.
>
> While verifying upgrade of subscriber patch, I found one issue with
> upgrade in verbose mode.
> I was able to reproduce this issue by performing a upgrade with a
> verbose option.
>
> The trace for the same is given below:
> Program received signal SIGSEGV, Segmentation fault.
> __strlen_sse2 () at ../sysdeps/x86_64/multiarch/strlen-vec.S:126
> 126        ../sysdeps/x86_64/multiarch/strlen-vec.S: No such file or directory.
> (gdb) bt
> #0  __strlen_sse2 () at ../sysdeps/x86_64/multiarch/strlen-vec.S:126
> #1  0x000055555556f572 in dopr (target=0x7fffffffbb90,
> format=0x55555557859e "\", plugin: \"%s\", two_phase: %s",
> args=0x7fffffffdc40) at snprintf.c:444
> #2  0x000055555556ed95 in pg_vsnprintf (str=0x7fffffffbc10 "slot_name:
> \"ication slots within the database:", count=8192, fmt=0x555555578590
> "slot_name: \"%s\", plugin: \"%s\", two_phase: %s",
>     args=0x7fffffffdc40) at snprintf.c:195
> #3  0x00005555555667e3 in pg_log_v (type=PG_VERBOSE,
> fmt=0x555555578590 "slot_name: \"%s\", plugin: \"%s\", two_phase: %s",
> ap=0x7fffffffdc40) at util.c:184
> #4  0x0000555555566b38 in pg_log (type=PG_VERBOSE, fmt=0x555555578590
> "slot_name: \"%s\", plugin: \"%s\", two_phase: %s") at util.c:264
> #5  0x0000555555561a06 in print_slot_infos (slot_arr=0x555555595ed0)
> at info.c:813
> #6  0x000055555556186e in print_db_infos (db_arr=0x555555587518
> <new_cluster+120>) at info.c:782
> #7  0x00005555555606da in get_db_rel_and_slot_infos
> (cluster=0x5555555874a0 <new_cluster>, live_check=false) at info.c:308
> #8  0x000055555555839a in check_new_cluster () at check.c:215
> #9  0x0000555555563010 in main (argc=13, argv=0x7fffffffdf08) at
> pg_upgrade.c:136
>
> This issue occurs because we are accessing uninitialized slot array information.
>
> We could fix it by a couple of ways: a) Initialize the whole of
> dbinfos by using pg_malloc0 instead of pg_malloc which will ensure
> that the slot information is set to 0. b) Setting only slot
> information. Attached patch has the changes for both the approaches.
> Thoughts?

Here is a small improvisation where num_slots need not be initialized
as it will be used only after assigning the result now. The attached
patch has the changes for the same.

Regards,
Vignesh