Thread: 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
Attachment
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
Attachment
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 andThis stuff refers to SUBREL_STATE_* in pg_subscription_rel.h, and FWIWI find confusing the term "transient" in this context as a state maylast for a rather long time, depending on the time it takes tosynchronize the relation, no? I am wondering if we could do betterhere, say:"The state tracking the progress of the relation synchronization isadditionally stored in shared memory, with SYNCWAIT and CATCHUP onlyappearing in memory."
WFM.
--
Euler Taveira
On Tue, Feb 2, 2021 at 10:49 AM Michael Paquier <michael@paquier.xyz> 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.
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
Attachment
On Wed, Feb 3, 2021 at 6:13 PM Michael Paquier <michael@paquier.xyz> 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
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
Attachment
On Wed, Feb 3, 2021 at 1:35 PM Michael Paquier <michael@paquier.xyz> 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.
On Wed, Feb 3, 2021 at 11:55 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Wed, Feb 3, 2021 at 1:35 PM Michael Paquier <michael@paquier.xyz> 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
Attachment
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