Horton

Thread

Commits

  1. Clarify comment in tablesync.c

  1. Typo in tablesync comment

    Peter Smith <smithpb2250@gmail.com> — 2021-02-01T23:38:31Z

    PSA a trivial patch to correct what seems like a typo in the tablesync comment.

----
Kind Regards,
Peter Smith.
Fujitsu Australia

  2. Re: Typo in tablesync comment

    Michael Paquier <michael@paquier.xyz> — 2021-02-02T05:19:01Z

    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

  3. Re: Typo in tablesync comment

    Euler Taveira <euler@eulerto.com> — 2021-02-02T11:27:54Z

    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/

  4. Re: Typo in tablesync comment

    Amit Kapila <amit.kapila16@gmail.com> — 2021-02-02T13:53:37Z

    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.

  5. Re: Typo in tablesync comment

    Michael Paquier <michael@paquier.xyz> — 2021-02-03T07:13:49Z

    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

  6. Re: Typo in tablesync comment

    Peter Smith <smithpb2250@gmail.com> — 2021-02-03T07:52:56Z

    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

  7. Re: Typo in tablesync comment

    Michael Paquier <michael@paquier.xyz> — 2021-02-03T08:04:59Z

    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

  8. Re: Typo in tablesync comment

    Amit Kapila <amit.kapila16@gmail.com> — 2021-02-03T12:55:19Z

    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.

  9. Re: Typo in tablesync comment

    Peter Smith <smithpb2250@gmail.com> — 2021-02-03T23:50:11Z

    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

  10. Re: Typo in tablesync comment

    Michael Paquier <michael@paquier.xyz> — 2021-02-04T07:08:43Z

    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