On Mon, Nov 14, 2022 at 6:52 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.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.
> Currently, IIUC we try to disable subscription regardless of the state, but
> should we avoid to reread catalog if workers are handling the transactions,
> like LogicalRepApplyLoop()?
>
IIUC, here you are referring to reading catalogs again via the
function maybe_reread_subscription(), right? If so, I think the idea
is to not invoke it frequently to avoid increasing transaction apply
time. However, when you are anyway going to wait for a delay, it may
not matter. I feel it would be better to add some comments saying that
we don't want workers to wait for a long time if users have disabled
the subscription or reduced the apply_delay time.
[1] - https://www.postgresql.org/message-id/CAA4eK1JRs0v9Z65HWKEZg3quWx4LiQ%3DpddTJZ_P1koXsbR3TMA%40mail.gmail.com
--
With Regards,
Amit Kapila.
