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
