On Mon, Nov 14, 2022 at 6:52 PM Hayato Kuroda (Fujitsu) <kuroda.hay...@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.
