Re: [HACKERS] Walsender timeouts and large transactions

On 29 November 2017 at 23:59, Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>
wrote:

> Hi,
>
> On 17/11/17 08:35, Kyotaro HORIGUCHI wrote:
> >
> > Moving around the code allow us to place ps_is_send_pending() in
> > the while condition, which seems to be more proper place to do
> > that. I haven't added test for this particular case.
> >
> > I tested this that
> >
> > - cleanly applies on the current master HEAD and passes make
> > check and subscription test.
> >
> > - walsender properly chooses the slow-path even if
> > pq_is_send_pending() is always false. (happens on a fast enough
> > network)
> >
> > - walsender waits properly waits on socket and process-reply time
> > in WaitLatchOrSocket.
> >
> > - walsender exits by timeout on network stall.
> >
> > So, I think the patch is functionally perfect.
> >
> > I'm a reviewer of this patch but I think I'm not allowed to mark
> > this "Ready for Commiter" since the last change is made by me.
> >
>
> Thanks for working on this, but there are couple of problems with your
> modifications which mean that it does not actually fix the original
> issue anymore (transaction taking long time to decode while sending
> changes over network works fine will result in walsender timout).
>
> The firs one is that you put pq_is_send_pending() in the while so the
> while is again never executed if there is no network send pending which
> makes the if above meaningless. Also you missed ProcessRepliesIfAny()
> when moving code around. That's needed for timeout calculations to work
> correctly.
>
> So one more revision attached with those things fixed. This version
> fixes the original issue as well.
>
>
I'm happy with what I see here. Commit message needs tweaking, but any
committer would do that anyway.

To me it looks like it's time to get this committed, marking as such.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services