Re: [HACKERS] Walsender timeouts and large transactions

From: Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, Craig Ringer <craig(at)2ndquadrant(dot)com>
Cc: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, Yura Sokolov <funny(dot)falcon(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Walsender timeouts and large transactions
Date: 2017-12-06 17:22:42
Message-ID: 6f44fc5e-3083-3fe0-4078-fa6e94133d3f@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On 05/12/17 21:07, Robert Haas wrote:
> On Mon, Dec 4, 2017 at 10:59 PM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
>> To me it looks like it's time to get this committed, marking as such.
>
> This version has noticeably more code rearrangement than before, and
> I'm not sure that is actually buying us anything. Why not keep the
> changes minimal?
>

Yeah we moved things around in the end, the main reason would be that it
actually works closer to how it was originally designed to work. Meaning
that the slow path is not so slow when !pq_is_send_pending() which seems
to have been the reasoning for original coding.

It's not completely necessary to do it for fixing the bug, but why make
things slower than they need to be.

> Also, TBH, this doesn't seem to have been carefully reviewed for style:
>
> - if (!pq_is_send_pending())
> - return;
> + /* Try taking fast path unless we get too close to walsender timeout. */
> + if (now < TimestampTzPlusMilliseconds(last_reply_timestamp,
> + wal_sender_timeout / 2))
> + {
> + if (!pq_is_send_pending())
> + return;
> + }
>
> Generally we write if (a && b) { ... } not if (a) { if (b) .. }
>

It's rather ugly with && because one of the conditions is two line, but
okay here you go. I am keeping the brackets even if normally don't for
one-liners because it's completely unreadable without them IMHO.

> - }
> + };
>

Oops.

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

Attachment Content-Type Size
v6-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 Adam Brusselback 2017-12-06 17:24:19 Re: Postgres with pthread
Previous Message Andres Freund 2017-12-06 17:17:37 Re: Postgres with pthread