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

Hi,

On Friday, February 10, 2023 2:05 PM Friday, February 10, 2023 2:05 PM wrote:
> On Fri, Feb 10, 2023 at 10:11 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> >
> > On Fri, Feb 10, 2023 at 6:27 AM Kyotaro Horiguchi
> > <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> > >
> > > At Thu, 9 Feb 2023 13:48:52 +0530, Amit Kapila
> > > <amit(dot)kapila16(at)gmail(dot)com> wrote in
> > > > On Thu, Feb 9, 2023 at 10:45 AM Kyotaro Horiguchi
> > > > <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> > > > >
> > > > > At Wed, 8 Feb 2023 09:03:03 +0000, "Hayato Kuroda (Fujitsu)"
> > > > > <kuroda(dot)hayato(at)fujitsu(dot)com> wrote in
> > > > > > Thank you for reviewing! PSA new version.
> > > > >
> > > > > + if (statusinterval_ms > 0 && diffms >
> > > > > + statusinterval_ms)
> > > > >
> > > > > The next expected feedback time is measured from the last status
> > > > > report. Thus, it seems to me this may suppress feedbacks from
> > > > > being sent for an unexpectedly long time especially when
> > > > > min_apply_delay is shorter than wal_r_s_interval.
> > > > >
> > > >
> > > > I think the minimum time before we send any feedback during the
> > > > wait is wal_r_s_interval. Now, I think if there is no transaction
> > > > for a long time before we get a new transaction, there should be
> > > > keep-alive messages in between which would allow us to send
> > > > feedback at regular intervals (wal_receiver_status_interval). So,
> > > > I think we should be
> > >
> > > Right.
> > >
> > > > able to send feedback in less than 2 *
> > > > wal_receiver_status_interval unless wal_sender/receiver timeout is
> > > > very large and there is a very low volume of transactions. Now, we
> > > > can try to send the feedback
> > >
> > > We have suffered this kind of feedback silence many times. Thus I
> > > don't want to rely on luck here. I had in mind of exposing last_send
> > > itself or providing interval-calclation function to the logic.
> > >
> >
> > I think we have last_send time in send_feedback(), so we can expose it
> > if we want but how would that solve the problem you are worried about?
> >
>
> I have an idea to use last_send time to avoid walsenders being timeout.
> Instead of directly using wal_receiver_status_interval as a minimum interval
> to send the feedback, we should check if it is greater than last_send time
> then we should send the feedback after (wal_receiver_status_interval -
> last_send). I think they can probably be different only on the very first time.
> Any better ideas?
This idea sounds good to me and
implemented this idea in an attached patch v34.

In the previous patch, we couldn't solve the
timeout of the publisher, when we conduct a scenario suggested by Horiguchi-san
and reproduced in the scenario attached test file 'test.sh'.
But now we handle it by adjusting the timing of the first wait time.

FYI, we thought to implement the new variable 'send_time'
in the LogicalRepWorker structure at first. But, this structure
is used when launcher controls workers or reports statistics
and it stores TimestampTz recorded in the received WAL,
so not sure if the struct is the right place to implement the variable.
Moreover, there are other similar variables such as last_recv_time
or reply_time. So, those will be confusing when we decide to have
new variable together. Then, it's declared separately.

The new patch also includes some changes for wait event.
Kindly have a look at the v34 patch.

Best Regards,
Takamichi Osumi