Re: Single transaction in the tablesync worker?

Thankyou for the feedback.

On Thu, Jan 7, 2021 at 12:45 PM Hou, Zhijie <houzj.fnst@cn.fujitsu.com> wrote:
>
> > PSA the v11 patch for the Tablesync Solution1.
> >
> > Difference from v10:
> > - Addresses several recent review comments.
> > - pg_indent has been run
> >
> Hi
>
> I took a look into the patch and have some comments.
>
> 1.
>   *       So the state progression is always: INIT -> DATASYNC -> SYNCWAIT ->
> - *       CATCHUP -> SYNCDONE -> READY.
> + *       CATCHUP -> (sync worker TCOPYDONE) -> SYNCDONE -> READY.
>
> I noticed the new state TCOPYDONE is commented between CATCHUP and SYNCDONE,
> But It seems the SUBREL_STATE_TCOPYDONE is actually set before SUBREL_STATE_SYNCWAIT[1].
> Did i miss something here ?
>
> [1]-----------------
> +       UpdateSubscriptionRelState(MyLogicalRepWorker->subid,
> +                                                          MyLogicalRepWorker->relid,
> +                                                          SUBREL_STATE_TCOPYDONE,
> +                                                          MyLogicalRepWorker->relstate_lsn);
> ...
>         /*
>          * We are done with the initial data synchronization, update the state.
>          */
>         SpinLockAcquire(&MyLogicalRepWorker->relmutex);
>         MyLogicalRepWorker->relstate = SUBREL_STATE_SYNCWAIT;
> ------------------
>

Thanks for reporting this mistake. I will correct the comment for the
next patch (v12)

>
> 2.
>         <literal>i</literal> = initialize,
>         <literal>d</literal> = data is being copied,
> +       <literal>C</literal> = table data has been copied,
>         <literal>s</literal> = synchronized,
>         <literal>r</literal> = ready (normal replication)
>
> +#define SUBREL_STATE_TCOPYDONE 't' /* tablesync copy phase is completed
> +                                                                        * (sublsn NULL) */
> The character representing 'data has been copied' in the catalog seems different from the macro define.
>

Yes, same was already previously reported [1]
[1] https://www.postgresql.org/message-id/CAA4eK1Kyi037XZzyrLE71MS2KoMmNSqa6RrQLdSCeeL27gnL%2BA%40mail.gmail.com
It will be fixed in the next patch (v12)

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