On Tue, Aug 9, 2022 at 3:52 AM Euler Taveira <euler@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.
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?
--
With Regards,
Amit Kapila.