On Wed, Nov 1, 2017 at 8:19 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.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
theprotocol 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
