RE: Single transaction in the tablesync worker?

> 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;
------------------

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.

Best regards,
houzj