Hi,
On Thursday, August 11, 2022 7:33 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > On Tue, Aug 9, 2022 at 3:52 AM Euler Taveira <eu...@eulerto.com> wrote: > > > > On Wed, Aug 3, 2022, at 10:27 AM, Amit Kapila wrote: > > > > Your explanation makes sense to me. The other point to consider is > > that there can be cases where we may not apply operation for the > > transaction because of empty transactions (we don't yet skip empty > > xacts for prepared transactions). So, won't it be better to apply the > > delay just before we apply the first change for a transaction? Do we > > want to apply the delay during table sync as we sometimes do need to > > enter apply phase while doing table sync? > > > > I thought about the empty transactions but decided to not complicate > > the code mainly because skipping transactions is not a code path that > > will slow down this feature. As explained in the documentation, there > > is no harm in delaying a transaction for more than min_apply_delay; it > > cannot apply earlier. Having said that I decided to do nothing. I'm > > also not sure if it deserves a comment or if this email is a possible > > explanation > for this decision. > > > > I don't know what makes you think it will complicate the code. But anyway > thinking further about the way apply_delay is used at various places in the > patch, > as pointed out by Peter Smith it seems it won't work for the parallel apply > feature where we start applying the transaction immediately after start > stream. > I was wondering why don't we apply delay after each commit of the transaction > rather than at the begin command. We can remember if the transaction has > made any change and if so then after commit, apply the delay. If we can do > that > then it will alleviate the concern of empty and skipped xacts as well. I agree with this direction. I'll update this point in a subsequent patch. > Another thing I was wondering how to determine what is a good delay time for > tests and found that current tests in replay_delay.pl uses 3s, so should we > use > the same for apply delay tests in this patch as well? Fixed in the patch posted in [1]. [1] - https://www.postgresql.org/message-id/TYCPR01MB8373775ECC6972289AF8CB30ED0F9%40TYCPR01MB8373.jpnprd01.prod.outlook.com Best Regards, Takamichi Osumi