Re: Typo in tablesync comment
On Thu, Feb 04, 2021 at 10:50:11AM +1100, Peter Smith wrote: > OK. I attached an updated patch using this new text. Thanks, applied. -- Michael signature.asc Description: PGP signature
Re: Typo in tablesync comment
On Wed, Feb 3, 2021 at 11:55 PM Amit Kapila wrote: > > On Wed, Feb 3, 2021 at 1:35 PM Michael Paquier wrote: > > > > On Wed, Feb 03, 2021 at 06:52:56PM +1100, Peter Smith wrote: > > > > > Maybe better to rewrite it more drastically? > > > > > > e.g > > > - > > > *The catalog pg_subscription_rel is used to keep information about > > > *subscribed tables and their state. The catalog holds all states > > > *except SYNCWAIT and CATCHUP which are only in shared memory. > > > - > > > > Fine by me. > > > > +1. > OK. I attached an updated patch using this new text. Kind Regards, Peter Smith. Fujitsu Australia v2-0001-Fix-tablesync-comment-typo.patch Description: Binary data
Re: Typo in tablesync comment
On Wed, Feb 3, 2021 at 1:35 PM Michael Paquier wrote: > > On Wed, Feb 03, 2021 at 06:52:56PM +1100, Peter Smith wrote: > > > Maybe better to rewrite it more drastically? > > > > e.g > > - > > *The catalog pg_subscription_rel is used to keep information about > > *subscribed tables and their state. The catalog holds all states > > *except SYNCWAIT and CATCHUP which are only in shared memory. > > - > > Fine by me. > +1. -- With Regards, Amit Kapila.
Re: Typo in tablesync comment
On Wed, Feb 03, 2021 at 06:52:56PM +1100, Peter Smith wrote: > OTOH I thought "additionally stored" made it seem like those states > were in the catalog and "additionally" in shared memory. Good point. > Maybe better to rewrite it more drastically? > > e.g > - > *The catalog pg_subscription_rel is used to keep information about > *subscribed tables and their state. The catalog holds all states > *except SYNCWAIT and CATCHUP which are only in shared memory. > - Fine by me. -- Michael signature.asc Description: PGP signature
Re: Typo in tablesync comment
On Wed, Feb 3, 2021 at 6:13 PM Michael Paquier wrote: > > On Tue, Feb 02, 2021 at 07:23:37PM +0530, Amit Kapila wrote: > > I don't mind changing to your proposed text but I think the current > > wording is also okay and seems clear to me. > > Reading that again, I still find the word "transient" to be misleading > in this context. Any extra opinions? OTOH I thought "additionally stored" made it seem like those states were in the catalog and "additionally" in shared memory. Maybe better to rewrite it more drastically? e.g - *The catalog pg_subscription_rel is used to keep information about *subscribed tables and their state. The catalog holds all states *except SYNCWAIT and CATCHUP which are only in shared memory. - Kind Regards, Peter Smith. Fujitsu Australia
Re: Typo in tablesync comment
On Tue, Feb 02, 2021 at 07:23:37PM +0530, Amit Kapila wrote: > I don't mind changing to your proposed text but I think the current > wording is also okay and seems clear to me. Reading that again, I still find the word "transient" to be misleading in this context. Any extra opinions? -- Michael signature.asc Description: PGP signature
Re: Typo in tablesync comment
On Tue, Feb 2, 2021 at 10:49 AM Michael Paquier wrote: > > On Tue, Feb 02, 2021 at 10:38:31AM +1100, Peter Smith wrote: > > PSA a trivial patch to correct what seems like a typo in the tablesync > > comment. > > - * subscribed tables and their state. Some transient state during data > - * synchronization is kept in shared memory. The states SYNCWAIT and > + * subscribed tables and their state. Some transient states during > data > + * synchronization are kept in shared memory. The states SYNCWAIT and > > This stuff refers to SUBREL_STATE_* in pg_subscription_rel.h, and FWIW > I find confusing the term "transient" in this context as a state may > last for a rather long time, depending on the time it takes to > synchronize the relation, no? > These in-memory states are used after the initial copy is done. So, these are just for the time the tablesync worker is synced-up with apply worker. In some cases, they could be for a longer period of time when apply worker is quite ahead of tablesync worker then we will be in the CATCHUP state for a long time but SYNCWAIT will still be for a shorter period of time. > I am wondering if we could do better > here, say: > "The state tracking the progress of the relation synchronization is > additionally stored in shared memory, with SYNCWAIT and CATCHUP only > appearing in memory." > I don't mind changing to your proposed text but I think the current wording is also okay and seems clear to me. -- With Regards, Amit Kapila.
Re: Typo in tablesync comment
On Tue, Feb 2, 2021, at 2:19 AM, Michael Paquier wrote: > On Tue, Feb 02, 2021 at 10:38:31AM +1100, Peter Smith wrote: > > PSA a trivial patch to correct what seems like a typo in the tablesync > > comment. > > - * subscribed tables and their state. Some transient state during data > - * synchronization is kept in shared memory. The states SYNCWAIT and > + * subscribed tables and their state. Some transient states during > data > + * synchronization are kept in shared memory. The states SYNCWAIT and > > This stuff refers to SUBREL_STATE_* in pg_subscription_rel.h, and FWIW > I find confusing the term "transient" in this context as a state may > last for a rather long time, depending on the time it takes to > synchronize the relation, no? I am wondering if we could do better > here, say: > "The state tracking the progress of the relation synchronization is > additionally stored in shared memory, with SYNCWAIT and CATCHUP only > appearing in memory." WFM. -- Euler Taveira EDB https://www.enterprisedb.com/
Re: Typo in tablesync comment
On Tue, Feb 02, 2021 at 10:38:31AM +1100, Peter Smith wrote: > PSA a trivial patch to correct what seems like a typo in the tablesync > comment. - * subscribed tables and their state. Some transient state during data - * synchronization is kept in shared memory. The states SYNCWAIT and + * subscribed tables and their state. Some transient states during data + * synchronization are kept in shared memory. The states SYNCWAIT and This stuff refers to SUBREL_STATE_* in pg_subscription_rel.h, and FWIW I find confusing the term "transient" in this context as a state may last for a rather long time, depending on the time it takes to synchronize the relation, no? I am wondering if we could do better here, say: "The state tracking the progress of the relation synchronization is additionally stored in shared memory, with SYNCWAIT and CATCHUP only appearing in memory." -- Michael signature.asc Description: PGP signature
Typo in tablesync comment
PSA a trivial patch to correct what seems like a typo in the tablesync comment. Kind Regards, Peter Smith. Fujitsu Australia fix-tablesync-comment-typo-20210202.patch Description: Binary data
