Re: Time delayed LR (WAS Re: logical replication restrictions)

On Tue, Nov 15, 2022 at 12:33 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Nov 14, 2022 at 6:52 PM Hayato Kuroda (Fujitsu)
> <kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
> >
> > Dear Amit,
> >
> > > > > It seems to me Kuroda-San has proposed this change [1] to fix the test
> > > > > but it is not clear to me why such a change is required. Why can't
> > > > > CHECK_FOR_INTERRUPTS() after waiting, followed by the existing below
> > > > > code [2] in LogicalRepApplyLoop() sufficient to handle parameter
> > > > > updates?
> >
> > (I forgot to say, this change was not proposed by me. I said that there should be
> > modified. I thought workers should wake up after the transaction was committed.)
> >
> > > So, why only honor the 'disable' option of the subscription? For
> > > example, one can change 'min_apply_delay' and it seems
> > > recoveryApplyDelay() honors a similar change in the recovery
> > > parameter. Is there a way to set the latch of the worker process, so
> > > that it can recheck if anything is changed?
> >
> > I have not considered about it, but seems reasonable. We may be able to
> > do maybe_reread_subscription() if subscription parameters are changed
> > and latch is set.
> >
>
> One more thing I would like you to consider is the point raised by me
> related to this patch's interaction with the parallel apply feature as
> mentioned in the email [1]. I am not sure the idea proposed in that
> email [1] is a good one because delaying after applying commit may not
> be good as we want to delay the apply of the transaction(s) on
> subscribers by this feature. I feel this needs more thought.
>

I have thought a bit more about this and we have the following options
to choose the delay point from. (a) apply delay just before committing
a transaction. As mentioned in comments in the patch this can lead to
bloat and locks held for a long time. (b) apply delay before starting
to apply changes for a transaction but here the problem is which time
to consider. In some cases, like for streaming transactions, we don't
receive the commit/prepare xact time in the start message. (c) use (b)
but use the previous transaction's commit time. (d) apply delay after
committing a transaction by using the xact's commit time.

At this stage, among above, I feel any one of (c) or (d) is worth
considering. Now, the difference between (c) and (d) is that if after
commit the next xact's data is already delayed by more than
min_apply_delay time then we don't need to kick the new logic of apply
delay.

The other thing to consider whether we need to process any keepalive
messages during the delay because otherwise, walsender may think that
the subscriber is not available and time out. This may not be a
problem for synchronous replication but otherwise, it could be a
problem.

Thoughts?

--
With Regards,
Amit Kapila.