Re: [HACKERS] Walsender timeouts and large transactions

From: Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>
To: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: robertmhaas(at)gmail(dot)com, funny(dot)falcon(at)postgrespro(dot)ru, pgsql-hackers(at)postgresql(dot)org, pjmodos(at)pjmodos(dot)net
Subject: Re: [HACKERS] Walsender timeouts and large transactions
Date: 2017-11-29 15:59:17
Message-ID: 86acff25-ca5d-1dc0-2da7-006344021f06@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

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.

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

Attachment Content-Type Size
v5-0001-Fix-walsender-timeouts-when-decoding-large-transacti.patch text/x-patch 4.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-11-29 16:00:32 Re: using index or check in ALTER TABLE SET NOT NULL
Previous Message Stephen Frost 2017-11-29 15:58:12 Re: using index or check in ALTER TABLE SET NOT NULL