Thankyou for the feedback. On Thu, Jan 7, 2021 at 12:45 PM Hou, Zhijie <houzj.f...@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.