On Tue, 7 Nov 2023 at 13:25, Amit Kapila <amit.kapil...@gmail.com> wrote:
>
> On Tue, Nov 7, 2023 at 10:01 AM Zhijie Hou (Fujitsu)
> <houzj.f...@fujitsu.com> wrote:
> >
> > On Tuesday, November 7, 2023 12:14 PM Kuroda, Hayato/黒田 隼人
> > <kuroda.hay...@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?
Regards,
Vignesh
diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index 7f21d26fd2..d2a1815fef 100644
--- a/src/bin/pg_upgrade/info.c
+++ b/src/bin/pg_upgrade/info.c
@@ -408,7 +408,7 @@ get_db_infos(ClusterInfo *cluster)
i_spclocation = PQfnumber(res, "spclocation");
ntups = PQntuples(res);
- dbinfos = (DbInfo *) pg_malloc(sizeof(DbInfo) * ntups);
+ dbinfos = (DbInfo *) pg_malloc0(sizeof(DbInfo) * ntups);
for (tupnum = 0; tupnum < ntups; tupnum++)
{
@@ -640,11 +640,7 @@ get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check)
/* Logical slots can be migrated since PG17. */
if (GET_MAJOR_VERSION(old_cluster.major_version) <= 1600)
- {
- dbinfo->slot_arr.slots = slotinfos;
- dbinfo->slot_arr.nslots = num_slots;
return;
- }
conn = connectToServer(&old_cluster, dbinfo->db_name);
diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index 7f21d26fd2..21a0b0551a 100644
--- a/src/bin/pg_upgrade/info.c
+++ b/src/bin/pg_upgrade/info.c
@@ -297,6 +297,11 @@ get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check)
*/
if (cluster == &old_cluster)
get_old_cluster_logical_slot_infos(pDbInfo, live_check);
+ else
+ {
+ pDbInfo->slot_arr.slots = NULL;
+ pDbInfo->slot_arr.nslots = 0;
+ }
}
if (cluster == &old_cluster)
