Re: Question about WalSndWriteData

On 21.03.2018 10:08, Konstantin Knizhnik wrote:
>
>
> On 21.03.2018 04:50, Peter Eisentraut wrote:
>> On 3/16/18 12:08, Konstantin Knizhnik wrote:
>>> pq_putmessage_noblock copies data from ctx->out buffer to libpq
>>> buffers.
>>> After it we write timestamp to ctx->out buffer.
>>> And comments says that we should do it "as late as possible".
>>> But this timestamp is not included in the copy data packet which is
>>> already copied to libpq connection buffer.
>> There is a pq_flush_if_writable() right after this that will write out
>> the rest of ctx->out.
>>
> Sorry, But PQ_flush_if_writable calls socket_flush_if_writable (or
> mq_flush_if_writable).
> This function flushes pqlib connection buffer, i.e. PqSendBuffer.
> This buffer has no relation to ctx->out_buffer, where timestamp is
> written.
>
> The obvious fix is to move assignment of timestamp prior to
> pq_putmessage_noblock:
>
>
> WalSndWriteData(LogicalDecodingContext *ctx, XLogRecPtr lsn,
> TransactionId xid,
>                 bool last_write)
> {
>     TimestampTz    now;
>
>     /*
>      * 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);
>     now = GetCurrentTimestamp();
>     pq_sendint64(&tmpbuf, now);
>     memcpy(&ctx->out->data[1 + sizeof(int64) + sizeof(int64)],
>            tmpbuf.data, sizeof(int64));
>
>     /* output previously gathered data in a CopyData packet */
>     pq_putmessage_noblock('d', ctx->out->data, ctx->out->len);
>
>
>

Sorry, I have not received confirmation whether it is a bug or not and
is it going to be fixed.

--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company