From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com> |
Cc: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, Sokolov Yura <funny(dot)falcon(at)postgrespro(dot)ru>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Petr Jelinek <pjmodos(at)pjmodos(dot)net> |
Subject: | Re: Walsender timeouts and large transactions |
Date: | 2017-11-02 09:00:36 |
Message-ID: | CA+Tgmoasw8QT4dswwybCppHLyZcszdUgc-0ZjbasWWY-+g9aPw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Nov 1, 2017 at 8:19 PM, Petr Jelinek
<petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:
> sorry for the delay but I didn't have much time in past weeks to follow
> this thread.
+ TimestampTz now = GetCurrentTimestamp();
+
/* output previously gathered data in a CopyData packet */
pq_putmessage_noblock('d', ctx->out->data, ctx->out->len);
/*
* Fill the send timestamp last, so that it is taken as late as possible.
* This is somewhat ugly, but the protocol is set as it's already used for
* several releases by streaming physical replication.
*/
resetStringInfo(&tmpbuf);
- pq_sendint64(&tmpbuf, GetCurrentTimestamp());
+ pq_sendint64(&tmpbuf, now);
memcpy(&ctx->out->data[1 + sizeof(int64) + sizeof(int64)],
tmpbuf.data, sizeof(int64));
This change falsifies the comments. Maybe initialize now just after
resetSetringInfo() is done.
- /* fast path */
- /* Try to flush pending output to the client */
- if (pq_flush_if_writable() != 0)
- WalSndShutdown();
+ /* Try taking fast path unless we get too close to walsender timeout. */
+ if (now < TimestampTzPlusMilliseconds(last_reply_timestamp,
+ wal_sender_timeout / 2))
+ {
+ CHECK_FOR_INTERRUPTS();
- if (!pq_is_send_pending())
- return;
+ /* Try to flush pending output to the client */
+ if (pq_flush_if_writable() != 0)
+ WalSndShutdown();
+
+ if (!pq_is_send_pending())
+ return;
+ }
I think it's only the if (!pq_is_send_pending()) return; that needs to
be conditional here, isn't it? The pq_flush_if_writable() can be done
unconditionally.
Other than that this looks like a reasonable change to me, but I'm not
an expert on this code.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2017-11-02 09:01:54 | Re: [POC] hash partitioning |
Previous Message | Simon Riggs | 2017-11-02 08:44:44 | Re: Statement-level rollback |