Thread: New standby_slot_names GUC in PG 17
The release notes have this item:
Allow specification of physical standbys that must be synchronized
before they are visible to subscribers (Hou Zhijie, Shveta Malik)
The new server variable is standby_slot_names.
Is standby_slot_names an accurate name for this GUC? It seems too
generic.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
On Fri, Jun 21, 2024 at 11:37:54AM -0400, Bruce Momjian wrote: > The release notes have this item: > > Allow specification of physical standbys that must be synchronized > before they are visible to subscribers (Hou Zhijie, Shveta Malik) > > The new server variable is standby_slot_names. > > Is standby_slot_names an accurate name for this GUC? It seems too > generic. +1, I was considering bringing this up, too. I'm still thinking of alternate names to propose, though. -- nathan
Hi,
A humble input, as on primary we have #primary_slot_name = '' then should not it be okay to have standby_slot_names or standby_slot_name ? It seems consistent with the Guc on primary.
Another suggestion is standby_replication_slots.
Regards,
Muhammad Ikram
Bitnine Global.
On Fri, Jun 21, 2024 at 8:47 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
On Fri, Jun 21, 2024 at 11:37:54AM -0400, Bruce Momjian wrote:
> The release notes have this item:
>
> Allow specification of physical standbys that must be synchronized
> before they are visible to subscribers (Hou Zhijie, Shveta Malik)
>
> The new server variable is standby_slot_names.
>
> Is standby_slot_names an accurate name for this GUC? It seems too
> generic.
+1, I was considering bringing this up, too. I'm still thinking of
alternate names to propose, though.
--
nathan
--
Muhammad Ikram
Muhammad Ikram <mmikram@gmail.com> writes:
> A humble input, as on primary we have #primary_slot_name = '' then should
> not it be okay to have standby_slot_names or standby_slot_name ? It seems
> consistent with the Guc on primary.
> Another suggestion is *standby_replication_slots*.
IIUC, Bruce's complaint is that the name is too generic (which I agree
with). Given the stated functionality:
>>>> Allow specification of physical standbys that must be synchronized
>>>> before they are visible to subscribers (Hou Zhijie, Shveta Malik)
it seems like the name ought to have some connection to
synchronization. Perhaps something like "synchronized_standby_slots"?
I haven't read the patch, so I don't know if this name is especially
on-point. But "standby_slot_names" seems completely unhelpful, as
a server could well have slots that are for standbys but are not to
be included in this list.
regards, tom lane
Thanks Tom Lane. You are more insightful.
Regards,
Ikram
On Sat, Jun 22, 2024 at 12:50 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Muhammad Ikram <mmikram@gmail.com> writes:
> A humble input, as on primary we have #primary_slot_name = '' then should
> not it be okay to have standby_slot_names or standby_slot_name ? It seems
> consistent with the Guc on primary.
> Another suggestion is *standby_replication_slots*.
IIUC, Bruce's complaint is that the name is too generic (which I agree
with). Given the stated functionality:
>>>> Allow specification of physical standbys that must be synchronized
>>>> before they are visible to subscribers (Hou Zhijie, Shveta Malik)
it seems like the name ought to have some connection to
synchronization. Perhaps something like "synchronized_standby_slots"?
I haven't read the patch, so I don't know if this name is especially
on-point. But "standby_slot_names" seems completely unhelpful, as
a server could well have slots that are for standbys but are not to
be included in this list.
regards, tom lane
--
Muhammad Ikram
On Fri, Jun 21, 2024 at 03:50:00PM -0400, Tom Lane wrote: >>>>> Allow specification of physical standbys that must be synchronized >>>>> before they are visible to subscribers (Hou Zhijie, Shveta Malik) > > it seems like the name ought to have some connection to > synchronization. Perhaps something like "synchronized_standby_slots"? IMHO that might be a bit too close to synchronous_standby_names. But the name might not be the only issue, as there is a separate proposal [0] to add _another_ GUC to tie standby_slot_names to synchronous replication. I wonder if this could just be a Boolean parameter or if folks really have use-cases for both a list of synchronous standbys and a separate list of synchronous standbys for failover slots. [0] https://postgr.es/m/CA%2B-JvFtq6f7%2BwAwSdud-x0yMTeMejUhpkyid1Xa_VNpRd_-oPw%40mail.gmail.com -- nathan
On Sat, Jun 22, 2024 at 1:49 AM Nathan Bossart <nathandbossart@gmail.com> wrote: > > On Fri, Jun 21, 2024 at 03:50:00PM -0400, Tom Lane wrote: > >>>>> Allow specification of physical standbys that must be synchronized > >>>>> before they are visible to subscribers (Hou Zhijie, Shveta Malik) > > > > it seems like the name ought to have some connection to > > synchronization. Perhaps something like "synchronized_standby_slots"? > > IMHO that might be a bit too close to synchronous_standby_names. But the > name might not be the only issue, as there is a separate proposal [0] to > add _another_ GUC to tie standby_slot_names to synchronous replication. I > wonder if this could just be a Boolean parameter or if folks really have > use-cases for both a list of synchronous standbys and a separate list of > synchronous standbys for failover slots. > Both have separate functionalities. We need to wait for the standby's in synchronous_standby_names to be synced at the commit time whereas the standby's in the standby_slot_names doesn't have such a requirement. The standby's in the standby_slot_names are used by logical WAL senders such that they will send decoded changes to plugins only after the specified replication slots confirm receiving WAL. So, combining them doesn't sound advisable. -- With Regards, Amit Kapila.
On Sat, Jun 22, 2024 at 1:49 AM Nathan Bossart <nathandbossart@gmail.com> wrote: > > On Fri, Jun 21, 2024 at 03:50:00PM -0400, Tom Lane wrote: > >>>>> Allow specification of physical standbys that must be synchronized > >>>>> before they are visible to subscribers (Hou Zhijie, Shveta Malik) > > > > it seems like the name ought to have some connection to > > synchronization. Perhaps something like "synchronized_standby_slots"? > > IMHO that might be a bit too close to synchronous_standby_names. > Right, but better than the current one. The other possibility could be wait_for_standby_slots. -- With Regards, Amit Kapila.
On Sat, Jun 22, 2024 at 03:17:03PM +0530, Amit Kapila wrote: > On Sat, Jun 22, 2024 at 1:49 AM Nathan Bossart <nathandbossart@gmail.com> wrote: > > > > On Fri, Jun 21, 2024 at 03:50:00PM -0400, Tom Lane wrote: > > >>>>> Allow specification of physical standbys that must be synchronized > > >>>>> before they are visible to subscribers (Hou Zhijie, Shveta Malik) > > > > > > it seems like the name ought to have some connection to > > > synchronization. Perhaps something like "synchronized_standby_slots"? > > > > IMHO that might be a bit too close to synchronous_standby_names. > > > > Right, but better than the current one. The other possibility could be > wait_for_standby_slots. FYI, changing this GUC name could force an initdb because postgresql.conf would have the old name and removing the comment to change it would cause an error. Therefore, we should change it ASAP. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Bruce Momjian <bruce@momjian.us> writes:
> FYI, changing this GUC name could force an initdb because
> postgresql.conf would have the old name and removing the comment to
> change it would cause an error. Therefore, we should change it ASAP.
That's not reason for a forced initdb IMO. It's easily fixed by
hand.
At this point we're into the release freeze for beta2, so even
if we had consensus on a new name it should wait till after.
So I see no particular urgency to make a decision.
regards, tom lane
On Saturday, June 22, 2024 5:47 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Sat, Jun 22, 2024 at 1:49 AM Nathan Bossart > <nathandbossart@gmail.com> wrote: > > > > On Fri, Jun 21, 2024 at 03:50:00PM -0400, Tom Lane wrote: > > >>>>> Allow specification of physical standbys that must be > > >>>>> synchronized before they are visible to subscribers (Hou Zhijie, > > >>>>> Shveta Malik) > > > > > > it seems like the name ought to have some connection to > > > synchronization. Perhaps something like "synchronized_standby_slots"? > > > > IMHO that might be a bit too close to synchronous_standby_names. > > > > Right, but better than the current one. The other possibility could be > wait_for_standby_slots. I agree the current name seems too generic and the suggested ' synchronized_standby_slots ' is better than the current one. Some other ideas could be: synchronize_slots_on_standbys: it indicates that the standbys that enabled slot sync should be listed in this GUC. logical_replication_wait_slots: it means the logical replication(logical Walsender process) will wait for these slots to advance the confirm flush lsn before proceeding. Best Regards, Hou zj
On Tue, Jun 25, 2024 at 11:21 AM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> wrote: > > On Saturday, June 22, 2024 5:47 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > > > On Sat, Jun 22, 2024 at 1:49 AM Nathan Bossart > > <nathandbossart@gmail.com> wrote: > > > > > > On Fri, Jun 21, 2024 at 03:50:00PM -0400, Tom Lane wrote: > > > >>>>> Allow specification of physical standbys that must be > > > >>>>> synchronized before they are visible to subscribers (Hou Zhijie, > > > >>>>> Shveta Malik) > > > > > > > > it seems like the name ought to have some connection to > > > > synchronization. Perhaps something like "synchronized_standby_slots"? > > > > > > IMHO that might be a bit too close to synchronous_standby_names. > > > > > > > Right, but better than the current one. The other possibility could be > > wait_for_standby_slots. > > I agree the current name seems too generic and the suggested ' synchronized_standby_slots ' > is better than the current one. > > Some other ideas could be: > > synchronize_slots_on_standbys: it indicates that the standbys that enabled > slot sync should be listed in this GUC. > > logical_replication_wait_slots: it means the logical replication(logical > Walsender process) will wait for these slots to advance the confirm flush > lsn before proceeding. I feel that the name that has some connection to "logical replication" also sounds good. Let me add some ideas: - logical_replication_synchronous_standby_slots (might be too long) - logical_replication_synchronous_slots Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
On Tue, Jun 25, 2024 at 8:20 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > On Tue, Jun 25, 2024 at 11:21 AM Zhijie Hou (Fujitsu) > <houzj.fnst@fujitsu.com> wrote: > > > > I agree the current name seems too generic and the suggested ' synchronized_standby_slots ' > > is better than the current one. > > > > Some other ideas could be: > > > > synchronize_slots_on_standbys: it indicates that the standbys that enabled > > slot sync should be listed in this GUC. > > > > logical_replication_wait_slots: it means the logical replication(logical > > Walsender process) will wait for these slots to advance the confirm flush > > lsn before proceeding. > > I feel that the name that has some connection to "logical replication" > also sounds good. Let me add some ideas: > > - logical_replication_synchronous_standby_slots (might be too long) > - logical_replication_synchronous_slots > I see your point about keeping logical_replication in the name but that could also lead one to think that this list can contain logical slots. OTOH, there is some value in keeping '_standby_' in the name as that is more closely associated with physical standby's and this list contains physical slots corresponding to physical standby's. So, my preference is in order as follows: synchronized_standby_slots, wait_for_standby_slots, logical_replication_wait_slots, logical_replication_synchronous_slots, and logical_replication_synchronous_standby_slots. -- With Regards, Amit Kapila.
Hi, On Tue, Jun 25, 2024 at 10:24:41AM +0530, Amit Kapila wrote: > On Tue, Jun 25, 2024 at 8:20 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > > > On Tue, Jun 25, 2024 at 11:21 AM Zhijie Hou (Fujitsu) > > <houzj.fnst@fujitsu.com> wrote: > > > > > > I agree the current name seems too generic and the suggested ' synchronized_standby_slots ' > > > is better than the current one. > > > > > > Some other ideas could be: > > > > > > synchronize_slots_on_standbys: it indicates that the standbys that enabled > > > slot sync should be listed in this GUC. > > > > > > logical_replication_wait_slots: it means the logical replication(logical > > > Walsender process) will wait for these slots to advance the confirm flush > > > lsn before proceeding. > > > > I feel that the name that has some connection to "logical replication" > > also sounds good. Let me add some ideas: > > > > - logical_replication_synchronous_standby_slots (might be too long) > > - logical_replication_synchronous_slots > > > > I see your point about keeping logical_replication in the name but > that could also lead one to think that this list can contain logical > slots. Agree, and we may add the same functionality for physical replication slots in the future too (it has been discussed in the thread [1]). So I don't think "logical" should be part of the name. > OTOH, there is some value in keeping '_standby_' in the name as > that is more closely associated with physical standby's and this list > contains physical slots corresponding to physical standby's. So, my > preference is in order as follows: synchronized_standby_slots, > wait_for_standby_slots, logical_replication_wait_slots, > logical_replication_synchronous_slots, and > logical_replication_synchronous_standby_slots. > I like the idea of having "synchronize[d]" in the name as it makes think of the feature it is linked to [2]. The slots mentioned in this parameter are linked to the "primary_slot_name" parameter on the standby, so what about? synchronized_primary_slot_names It makes clear it is somehow linked to "primary_slot_name" and that we want them to be in sync. So I'd vote for (in that order); synchronized_primary_slot_names, synchronized_standby_slots [1]: https://www.postgresql.org/message-id/bb437218-73bc-34c3-b8fb-8c1be4ddaec9%40enterprisedb.com [2]: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=93db6cbda037f1be9544932bd9a785dabf3ff712 Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
On Tue, Jun 25, 2024 at 1:54 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Tue, Jun 25, 2024 at 8:20 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > > > On Tue, Jun 25, 2024 at 11:21 AM Zhijie Hou (Fujitsu) > > <houzj.fnst@fujitsu.com> wrote: > > > > > > I agree the current name seems too generic and the suggested ' synchronized_standby_slots ' > > > is better than the current one. > > > > > > Some other ideas could be: > > > > > > synchronize_slots_on_standbys: it indicates that the standbys that enabled > > > slot sync should be listed in this GUC. > > > > > > logical_replication_wait_slots: it means the logical replication(logical > > > Walsender process) will wait for these slots to advance the confirm flush > > > lsn before proceeding. > > > > I feel that the name that has some connection to "logical replication" > > also sounds good. Let me add some ideas: > > > > - logical_replication_synchronous_standby_slots (might be too long) > > - logical_replication_synchronous_slots > > > > I see your point about keeping logical_replication in the name but > that could also lead one to think that this list can contain logical > slots. Right. > OTOH, there is some value in keeping '_standby_' in the name as > that is more closely associated with physical standby's and this list > contains physical slots corresponding to physical standby's. Agreed. > So, my > preference is in order as follows: synchronized_standby_slots, > wait_for_standby_slots, logical_replication_wait_slots, > logical_replication_synchronous_slots, and > logical_replication_synchronous_standby_slots. I also prefer synchronized_standby_slots. From a different angle just for discussion, is it worth considering the term 'failover' since the purpose of this feature is to ensure a standby to be ready for failover in terms of logical replication? For example, failover_standby_slot_names? Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
On Tue, Jun 25, 2024 at 12:30 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > On Tue, Jun 25, 2024 at 1:54 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > > > > So, my > > preference is in order as follows: synchronized_standby_slots, > > wait_for_standby_slots, logical_replication_wait_slots, > > logical_replication_synchronous_slots, and > > logical_replication_synchronous_standby_slots. > > I also prefer synchronized_standby_slots. > > From a different angle just for discussion, is it worth considering > the term 'failover' since the purpose of this feature is to ensure a > standby to be ready for failover in terms of logical replication? For > example, failover_standby_slot_names? > I feel synchronized better indicates the purpose because we ensure such slots are synchronized before we process changes for logical failover slots. We already have a 'failover' option for logical slots which could make things confusing if we add 'failover' where physical slots need to be specified. -- With Regards, Amit Kapila.
On Tue, Jun 25, 2024 at 02:02:09PM +0530, Amit Kapila wrote: > On Tue, Jun 25, 2024 at 12:30 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote: >> On Tue, Jun 25, 2024 at 1:54 PM Amit Kapila <amit.kapila16@gmail.com> wrote: >> > So, my >> > preference is in order as follows: synchronized_standby_slots, >> > wait_for_standby_slots, logical_replication_wait_slots, >> > logical_replication_synchronous_slots, and >> > logical_replication_synchronous_standby_slots. >> >> I also prefer synchronized_standby_slots. >> >> From a different angle just for discussion, is it worth considering >> the term 'failover' since the purpose of this feature is to ensure a >> standby to be ready for failover in terms of logical replication? For >> example, failover_standby_slot_names? > > I feel synchronized better indicates the purpose because we ensure > such slots are synchronized before we process changes for logical > failover slots. We already have a 'failover' option for logical slots > which could make things confusing if we add 'failover' where physical > slots need to be specified. I'm fine with synchronized_standby_slots. -- nathan
On Tue, Jun 25, 2024 at 5:32 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Tue, Jun 25, 2024 at 12:30 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > > > On Tue, Jun 25, 2024 at 1:54 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > > > > > > > So, my > > > preference is in order as follows: synchronized_standby_slots, > > > wait_for_standby_slots, logical_replication_wait_slots, > > > logical_replication_synchronous_slots, and > > > logical_replication_synchronous_standby_slots. > > > > I also prefer synchronized_standby_slots. > > > > From a different angle just for discussion, is it worth considering > > the term 'failover' since the purpose of this feature is to ensure a > > standby to be ready for failover in terms of logical replication? For > > example, failover_standby_slot_names? > > > > I feel synchronized better indicates the purpose because we ensure > such slots are synchronized before we process changes for logical > failover slots. We already have a 'failover' option for logical slots > which could make things confusing if we add 'failover' where physical > slots need to be specified. Agreed. So +1 for synchronized_stnadby_slots. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
On Wednesday, June 26, 2024 9:40 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > On Tue, Jun 25, 2024 at 5:32 PM Amit Kapila <amit.kapila16@gmail.com> > wrote: > > > > On Tue, Jun 25, 2024 at 12:30 PM Masahiko Sawada > <sawada.mshk@gmail.com> wrote: > > > > > > On Tue, Jun 25, 2024 at 1:54 PM Amit Kapila <amit.kapila16@gmail.com> > wrote: > > > > > > > > > > > So, my > > > > preference is in order as follows: synchronized_standby_slots, > > > > wait_for_standby_slots, logical_replication_wait_slots, > > > > logical_replication_synchronous_slots, and > > > > logical_replication_synchronous_standby_slots. > > > > > > I also prefer synchronized_standby_slots. > > > > > > From a different angle just for discussion, is it worth considering > > > the term 'failover' since the purpose of this feature is to ensure a > > > standby to be ready for failover in terms of logical replication? > > > For example, failover_standby_slot_names? > > > > > > > I feel synchronized better indicates the purpose because we ensure > > such slots are synchronized before we process changes for logical > > failover slots. We already have a 'failover' option for logical slots > > which could make things confusing if we add 'failover' where physical > > slots need to be specified. > > Agreed. So +1 for synchronized_stnadby_slots. +1. Since there is a consensus on this name, I am attaching the patch to rename the GUC to synchronized_stnadby_slots. I have confirmed that the regression tests and pgindent passed for the patch. Best Regards, Hou zj Best Regards, Hou zj
Attachment
Hi,
On Wed, Jun 26, 2024 at 04:17:45AM +0000, Zhijie Hou (Fujitsu) wrote:
> On Wednesday, June 26, 2024 9:40 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Tue, Jun 25, 2024 at 5:32 PM Amit Kapila <amit.kapila16@gmail.com>
> > wrote:
> > >
> > > I feel synchronized better indicates the purpose because we ensure
> > > such slots are synchronized before we process changes for logical
> > > failover slots. We already have a 'failover' option for logical slots
> > > which could make things confusing if we add 'failover' where physical
> > > slots need to be specified.
> >
> > Agreed. So +1 for synchronized_stnadby_slots.
>
> +1.
>
> Since there is a consensus on this name, I am attaching the patch to rename
> the GUC to synchronized_stnadby_slots. I have confirmed that the regression
> tests and pgindent passed for the patch.
>
Thanks for the patch!
A few comments:
1 ====
In the commit message:
"
The standby_slot_names GUC is intended to allow specification of physical
standby slots that must be synchronized before they are visible to
subscribers
"
Not sure that wording is correct, if we feel the need to explain the GUC,
maybe repeat some wording from bf279ddd1c?
2 ====
Should we rename StandbySlotNamesConfigData too?
3 ====
Should we rename SlotExistsInStandbySlotNames too?
4 ====
Should we rename validate_standby_slots() too?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Wed, Jun 26, 2024 at 10:19 AM Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote: > > > 2 ==== > > Should we rename StandbySlotNamesConfigData too? > How about SyncStandbySlotsConfigData? > 3 ==== > > Should we rename SlotExistsInStandbySlotNames too? > Similarly SlotExistsInSyncStandbySlots? > 4 ==== > > Should we rename validate_standby_slots() too? > And validate_sync_standby_slots()? --- a/doc/src/sgml/release-17.sgml +++ b/doc/src/sgml/release-17.sgml @@ -1325,7 +1325,7 @@ Author: Michael Paquier <michael@paquier.xyz> <!-- Author: Amit Kapila <akapila@postgresql.org> -2024-03-08 [bf279ddd1] Introduce a new GUC 'standby_slot_names'. +2024-03-08 [bf279ddd1] Introduce a new GUC 'synchronized_standby_slots'. I am not sure if it is a good idea to change release notes in the same commit as the code change. I would prefer to do it in a separate commit. -- With Regards, Amit Kapila.
On Wed, Jun 26, 2024 at 11:39:45AM +0530, Amit Kapila wrote: > --- a/doc/src/sgml/release-17.sgml > +++ b/doc/src/sgml/release-17.sgml > @@ -1325,7 +1325,7 @@ Author: Michael Paquier <michael@paquier.xyz> > > <!-- > Author: Amit Kapila <akapila@postgresql.org> > -2024-03-08 [bf279ddd1] Introduce a new GUC 'standby_slot_names'. > +2024-03-08 [bf279ddd1] Introduce a new GUC 'synchronized_standby_slots'. > > I am not sure if it is a good idea to change release notes in the same > commit as the code change. I would prefer to do it in a separate > commit. The existing commits referenced cannot change, but it's surely OK to add a reference to the commit doing the rename for this item in the release notes, and update the release notes to reflect the new GUC name. Using two separate commits ensures that the correct reference about the rename is added to the release notes, so that's the correct thing to do, IMHO. -- Michael
Attachment
Hi, On Wed, Jun 26, 2024 at 11:39:45AM +0530, Amit Kapila wrote: > On Wed, Jun 26, 2024 at 10:19 AM Bertrand Drouvot > <bertranddrouvot.pg@gmail.com> wrote: > > > > > > 2 ==== > > > > Should we rename StandbySlotNamesConfigData too? > > > > How about SyncStandbySlotsConfigData? > > > 3 ==== > > > > Should we rename SlotExistsInStandbySlotNames too? > > > > Similarly SlotExistsInSyncStandbySlots? > > > 4 ==== > > > > Should we rename validate_standby_slots() too? > > > > And validate_sync_standby_slots()? > Thanks! All of the above proposal sound good to me. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
On Wednesday, June 26, 2024 12:49 PM Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote: > > Hi, > > On Wed, Jun 26, 2024 at 04:17:45AM +0000, Zhijie Hou (Fujitsu) wrote: > > On Wednesday, June 26, 2024 9:40 AM Masahiko Sawada > <sawada.mshk@gmail.com> wrote: > > > > > > On Tue, Jun 25, 2024 at 5:32 PM Amit Kapila > > > <amit.kapila16@gmail.com> > > > wrote: > > > > > > > > I feel synchronized better indicates the purpose because we ensure > > > > such slots are synchronized before we process changes for logical > > > > failover slots. We already have a 'failover' option for logical > > > > slots which could make things confusing if we add 'failover' where > > > > physical slots need to be specified. > > > > > > Agreed. So +1 for synchronized_stnadby_slots. > > > > +1. > > > > Since there is a consensus on this name, I am attaching the patch to > > rename the GUC to synchronized_stnadby_slots. I have confirmed that > > the regression tests and pgindent passed for the patch. > A few comments: Thanks for the comments! > 1 ==== > > In the commit message: > > " > The standby_slot_names GUC is intended to allow specification of physical > standby slots that must be synchronized before they are visible to > subscribers > " > > Not sure that wording is correct, if we feel the need to explain the GUC, maybe > repeat some wording from bf279ddd1c? I intentionally copied some words from release note of this GUC which was also part of the content in the initial email of this thread. I think it would be easy to understand than the original commit msg. But others may have different opinion, so I would leave the decision to the committer. (I adjusted a bit the word in this version). > > 2 ==== > > Should we rename StandbySlotNamesConfigData too? > > 3 ==== > > Should we rename SlotExistsInStandbySlotNames too? > > 4 ==== > > Should we rename validate_standby_slots() too? > Renamed these to the names suggested by Amit. Attach the v2 patch set which addressed above and removed the changes in release-17.sgml according to the comment from Amit. Best Regards, Hou zj
Attachment
Hi, On Wed, Jun 26, 2024 at 09:15:48AM +0000, Zhijie Hou (Fujitsu) wrote: > Renamed these to the names suggested by Amit. > > Attach the v2 patch set which addressed above and removed > the changes in release-17.sgml according to the comment from Amit. > Thanks! LGTM. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
On Wed, Jun 26, 2024 at 6:00 PM Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote: > > On Wed, Jun 26, 2024 at 09:15:48AM +0000, Zhijie Hou (Fujitsu) wrote: > > Renamed these to the names suggested by Amit. > > > > Attach the v2 patch set which addressed above and removed > > the changes in release-17.sgml according to the comment from Amit. > > > > Thanks! LGTM. > As per my reading of this thread, we have an agreement on changing the GUC name standby_slot_names to synchronized_standby_slots. I'll wait for a day and push the change unless someone thinks otherwise. -- With Regards, Amit Kapila.
On 21.06.24 17:37, Bruce Momjian wrote: > The release notes have this item: > > Allow specification of physical standbys that must be synchronized > before they are visible to subscribers (Hou Zhijie, Shveta Malik) > > The new server variable is standby_slot_names. > > Is standby_slot_names an accurate name for this GUC? It seems too > generic. This was possibly inspired by pg_failover_slots.standby_slot_names (which in turn came from pglogical.standby_slot_names). In those cases, you have some more context from the extension prefix. The new suggested names sound good to me.
On Wed, Jun 26, 2024 at 6:15 PM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> wrote: > > On Wednesday, June 26, 2024 12:49 PM Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote: > > > > Hi, > > > > On Wed, Jun 26, 2024 at 04:17:45AM +0000, Zhijie Hou (Fujitsu) wrote: > > > On Wednesday, June 26, 2024 9:40 AM Masahiko Sawada > > <sawada.mshk@gmail.com> wrote: > > > > > > > > On Tue, Jun 25, 2024 at 5:32 PM Amit Kapila > > > > <amit.kapila16@gmail.com> > > > > wrote: > > > > > > > > > > I feel synchronized better indicates the purpose because we ensure > > > > > such slots are synchronized before we process changes for logical > > > > > failover slots. We already have a 'failover' option for logical > > > > > slots which could make things confusing if we add 'failover' where > > > > > physical slots need to be specified. > > > > > > > > Agreed. So +1 for synchronized_stnadby_slots. > > > > > > +1. > > > > > > Since there is a consensus on this name, I am attaching the patch to > > > rename the GUC to synchronized_stnadby_slots. I have confirmed that > > > the regression tests and pgindent passed for the patch. > > A few comments: > > Thanks for the comments! > > > 1 ==== > > > > In the commit message: > > > > " > > The standby_slot_names GUC is intended to allow specification of physical > > standby slots that must be synchronized before they are visible to > > subscribers > > " > > > > Not sure that wording is correct, if we feel the need to explain the GUC, maybe > > repeat some wording from bf279ddd1c? > > I intentionally copied some words from release note of this GUC which was > also part of the content in the initial email of this thread. I think it > would be easy to understand than the original commit msg. But others may > have different opinion, so I would leave the decision to the committer. (I adjusted > a bit the word in this version). > > > > > 2 ==== > > > > Should we rename StandbySlotNamesConfigData too? > > > > 3 ==== > > > > Should we rename SlotExistsInStandbySlotNames too? > > > > 4 ==== > > > > Should we rename validate_standby_slots() too? > > > > Renamed these to the names suggested by Amit. > > Attach the v2 patch set which addressed above and removed > the changes in release-17.sgml according to the comment from Amit. > Thank you for updating the patch. The v2 patch looks good to me. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
On Thu, Jun 27, 2024 at 7:14 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > On Wed, Jun 26, 2024 at 6:15 PM Zhijie Hou (Fujitsu) > <houzj.fnst@fujitsu.com> wrote: > > Thank you for updating the patch. The v2 patch looks good to me. > Pushed. -- With Regards, Amit Kapila.
On Monday, July 1, 2024 6:45 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Thu, Jun 27, 2024 at 7:14 AM Masahiko Sawada > <sawada.mshk@gmail.com> wrote: > > > > On Wed, Jun 26, 2024 at 6:15 PM Zhijie Hou (Fujitsu) > > <houzj.fnst@fujitsu.com> wrote: > > > > Thank you for updating the patch. The v2 patch looks good to me. > > > > Pushed. Thanks! I am attaching another patch to modify the release note as discussed. Best Regards, Hou zj
Attachment
On Mon, Jul 1, 2024 at 6:01 PM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> wrote: > > Thanks! I am attaching another patch to modify the release note as discussed. > Pushed. -- With Regards, Amit Kapila.