Re: Synchronizing slots from primary to standby
От | shveta malik |
---|---|
Тема | Re: Synchronizing slots from primary to standby |
Дата | |
Msg-id | CAJpy0uCv71ynK7zFkKWy3wa7ua+wGw=zoGKy9w=Xq2sq4PqAFQ@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Synchronizing slots from primary to standby (Amit Kapila <amit.kapila16@gmail.com>) |
Ответы |
Re: Synchronizing slots from primary to standby
(shveta malik <shveta.malik@gmail.com>)
RE: Synchronizing slots from primary to standby ("Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>) |
Список | pgsql-hackers |
On Thu, Dec 14, 2023 at 4:40 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Thu, Dec 14, 2023 at 7:00 AM Peter Smith <smithpb2250@gmail.com> wrote: > > > > Hi, here are a few more review comments for the patch v47-0002 > > > > (plus my review comments of v45-0002 [1] are yet to be addressed) > > > > ====== > > 1. General > > > > For consistency and readability, try to use variables of the same > > names whenever they have the same purpose, even when they declared are > > in different functions. A few like this were already mentioned in the > > previous review but there are more I keep noticing. > > > > For example, > > 'slotnameChanged' in function, VERSUS 'primary_slot_changed' in the caller. > > > > > > ====== > > src/backend/replication/logical/slotsync.c > > > > 2. > > +/* > > + * > > + * Validates the primary server info. > > + * > > + * Using the specified primary server connection, it verifies whether > > the master > > + * is a standby itself and returns true in that case to convey the caller that > > + * we are on the cascading standby. > > + * But if master is the primary server, it goes ahead and validates > > + * primary_slot_name. It emits error if the physical slot in primary_slot_name > > + * does not exist on the primary server. > > + */ > > +static bool > > +validate_primary_info(WalReceiverConn *wrconn) > > > > 2b. > > IMO it is too tricky to have a function called "validate_xxx", when > > actually you gave that return value some special unintuitive meaning > > other than just validation. IMO it is always better for the returned > > value to properly match the function name so the expectations are very > > obvious. So, In this case, I think a better function signature would > > be like this: > > > > SUGGESTION > > > > static void > > validate_primary_info(WalReceiverConn *wrconn, bool *master_is_standby) > > > > or > > > > static void > > validate_primary_info(WalReceiverConn *wrconn, bool *am_cascading_standby) > > > > The terminology master_is_standby is a bit indirect for this usage, so > I would prefer the second one. Shall we name this function as > check_primary_info()? Additionally, can we rewrite the following > comment: "Using the specified primary server connection, check whether > we are cascading standby. It also validates primary_slot_info for > non-cascading-standbys.". > > -- > With Regards, > Amit Kapila. PFA v48. Changes are: 1) Addressed comments by Peter for v45-002 and v47-002 given in [1] and [2] respectively 2) Addressed comments by Amit for v47-002 given in [3], [4] 3) Addressed an old comment (#74 in [5]) of getting rid of header inclusion from tablesync.c when there was no code change in that file. Thanks Hou-san for working on this change. TODO: --Address the test comments in [1] for 050_standby_failover_slots_sync.pl --Review the feasibility of addressing one pending comment (comment 13 in [5]) of 'r'->'n' conversion. [1]: https://www.postgresql.org/message-id/CAHut%2BPtOc7J_n24HJ6f_dFWTuD3X2ApOByQzZf6jZz%2B0wb-ebQ%40mail.gmail.com [2]: https://www.postgresql.org/message-id/CAHut%2BPsvxs-%3Dj3aCpPVs3e4w78HndCdO-F4bLPzAX70%2BdgWUuQ%40mail.gmail.com [3]: https://www.postgresql.org/message-id/CAA4eK1L2ts%3DgfiF4aw7-DH8HWj29s08hVRq-Ff8%3DmjfdUXx8CA%40mail.gmail.com [4]: https://www.postgresql.org/message-id/CAA4eK1%2Bw9yv%2B4UZXhiDHZpGDfbeRHYDBu23FwsniS8sYUZeu1w%40mail.gmail.com [5]: https://www.postgresql.org/message-id/CAJpy0uDcOf5Hvk_CdCCAbfx9SY%2Bog%3D%3D%3DtgiuhWKzkYyqebui9g%40mail.gmail.com thanks Shveta
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Dilip KumarДата:
Сообщение: Re: SLRU optimization - configurable buffer pool and partitioning the SLRU lock