Hi,
thanks for checking.
On 02/11/17 10:00, Robert Haas wrote:
> 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 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.
Eh, right, I can do that.
>
> - /* 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.
>
Well, even the CHECK_FOR_INTERRUPTS() can be called unconditionally yes.
It just seems like it's needless call as we'll call both in for loop
anyway if we take the "slow" path. I admit it's not exactly big win
though. If you think it would improve readability I can move it.
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
