From: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
---|---|
To: | 'Peter Smith' <smithpb2250(at)gmail(dot)com> |
Cc: | "Wei Wang (Fujitsu)" <wangw(dot)fnst(at)fujitsu(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Subject: | RE: [PoC] pg_upgrade: allow to upgrade publisher node |
Date: | 2023-05-16 06:15:00 |
Message-ID: | TYAPR01MB58664CB26AAA51EACA1C9B33F5799@TYAPR01MB5866.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Dear Peter,
Thanks for reviewing! PSA new version patchset.
> 1. get_logical_slot_infos_per_db
>
> I noticed that the way this is coded, 'ntups' and 'num_slots' seems to
> have exactly the same meaning. IMO you can simplify this by removing
> 'ntups'.
>
> BEFORE
> + int ntups;
> + int num_slots = 0;
>
> SUGGESTION
> + int num_slots;
>
> ~
>
> BEFORE
> + ntups = PQntuples(res);
> +
> + if (ntups)
> + {
>
> SUGGESTION
> + num_slots = PQntuples(res);
> +
> + if (num_slots)
> + {
>
> ~
>
> BEFORE
> + slotinfos = (LogicalSlotInfo *) pg_malloc(sizeof(LogicalSlotInfo) * ntups);
>
> SUGGESTION
> + slotinfos = (LogicalSlotInfo *) pg_malloc(sizeof(LogicalSlotInfo) *
> num_slots);
>
> ~
>
> BEFORE
> + for (slotnum = 0; slotnum < ntups; slotnum++)
> + {
> + LogicalSlotInfo *curr = &slotinfos[num_slots++];
>
> SUGGESTION
> + for (slotnum = 0; slotnum < ntups; slotnum++)
> + {
> + LogicalSlotInfo *curr = &slotinfos[slotnum];
Right, fixed.
> 2. get_logical_slot_infos, print_slot_infos
>
> > >
> > > In another thread [1] I am posting some minor patch changes to the
> > > VERBOSE logging (changes to double-quotes and commas etc.). Please
> > > keep a watch on that thread because if gets pushed then this one will
> > > be impacted. e.g. your logging here ought also to include the same
> > > suggested double quotes.
> >
> > I thought it would be pushed soon, so the suggestion was included.
>
> OK, but I think you have accidentally missed adding similar new double
> quotes to all other VERBOSE logging in your patch.
>
> e.g. see get_logical_slot_infos:
> pg_log(PG_VER
BOSE, "Database: %s", pDbInfo->db_name);
>
Oh, I missed it. Fixed. I grepped patches and could not find other lines
which should be double-quoted.
In addition, I ran pgindent again for 0001.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachment | Content-Type | Size |
---|---|---|
v14-0001-pg_upgrade-Add-include-logical-replication-slots.patch | application/octet-stream | 32.7 KB |
v14-0002-Always-persist-to-disk-logical-slots-during-a-sh.patch | application/octet-stream | 4.8 KB |
v14-0003-pg_upgrade-Add-check-function-for-include-logica.patch | application/octet-stream | 5.7 KB |
v14-0004-Change-the-method-used-to-check-logical-replicat.patch | application/octet-stream | 7.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2023-05-16 06:16:51 | Re: Introduce WAIT_EVENT_EXTENSION and WAIT_EVENT_BUFFER_PIN |
Previous Message | Drouvot, Bertrand | 2023-05-16 06:10:20 | Re: Introduce WAIT_EVENT_EXTENSION and WAIT_EVENT_BUFFER_PIN |