Re: logical replication restrictions

Hi Euler,

I've some comments/questions about the latest version (v4) of your patch.

Firstly, I think the patch needs a rebase. CI currently cannot apply it [1].

22. src/test/subscription/t/032_apply_delay.pl
>
> I received the following error when trying to run these 'subscription'
> tests:
>
> t/032_apply_delay.pl ............... No such class log_location at
> t/032_apply_delay.pl line 49, near "my log_location"
> syntax error at t/032_apply_delay.pl line 49, near "my log_location ="
>

I'm having these errors too. Seems like some declarations are missing.

+ specified amount of time. If this value is specified without
>> units,
>
> + it is taken as milliseconds. The default is zero, adding no
>> delay.
>
> + </para>
>
> I'm also having an issue when I give min_apply_delay parameter without
units.
I expect that if I set min_apply_delay to 5000 (without any unit), it will
be interpreted as 5000 ms.

I tried:
postgres=# CREATE SUBSCRIPTION testsub CONNECTION 'dbname=postgres
port=5432' PUBLICATION testpub WITH (min_apply_delay=5000);

And logs showed:
2022-07-13 20:26:52.231 +03 [5422] LOG: logical replication apply delay:
4999999 ms
2022-07-13 20:26:52.231 +03 [5422] CONTEXT: processing remote data for
replication origin "pg_18126" during "BEGIN" in transaction 3152 finished
at 0/465D7A0

Looks like it starts from 5000000 ms instead of 5000 ms for me. If I state
the unit as ms, then it works correctly.

Lastly, I have a question about this delay during tablesync.
It's stated here that apply delays are not for initial tablesync.

<para>
>
> + The delay occurs only on WAL records for transaction begins and
>> after
>
> + the initial table synchronization. It is possible that the
>
> + replication delay between publisher and subscriber exceeds the
>> value
>
> + of this parameter, in which case no delay is added. Note that
>> the
>
> + delay is calculated between the WAL time stamp as written on
>
> + publisher and the current time on the subscriber. Delays in
>> logical
>
> + decoding and in transfer the transaction may reduce the actual
>> wait
>
> + time. If the system clocks on publisher and subscriber are not
>
> + synchronized, this may lead to apply changes earlier than
>> expected.
>
> + This is not a major issue because a typical setting of this
>> parameter
>
> + are much larger than typical time deviations between servers.
>
> + </para>
>
>
There might be a case where tablesync workers are in SYNCWAIT state and
waiting for apply worker to tell them to CATCHUP.
And if apply worker is waiting in apply_delay function, tablesync workers
will be stuck at SYNCWAIT state and this might delay tablesync at least
"min_apply_delay" amount of time or more.
Is it something we would want? What do you think?

[1] http://cfbot.cputube.org/patch_38_3581.log

Best,
Melih