Re: Fix typo about WalSndPrepareWrite

At Thu, 14 Jan 2021 06:46:35 +0000, Li Japin <japinli(at)hotmail(dot)com> wrote in
>
> On Jan 14, 2021, at 12:56 PM, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com<mailto:ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>> wrote:
>
> Hi Japin,
> Thanks for the report.
>
> I think that comment is correct. It refers to the following code
> blocks of XLogSendPhysical()
>
> 2744 /*
> 2745 * OK to read and send the slice.
> 2746 */
> 2747 resetStringInfo(&output_message);
> 2748 pq_sendbyte(&output_message, 'w');
> 2749
> 2750 pq_sendint64(&output_message, startptr); /* dataStart */
> 2751 pq_sendint64(&output_message, SendRqstPtr); /* walEnd */
> 2752 pq_sendint64(&output_message, 0); /* sendtime, filled in last */
>
> 2803 * Fill the send timestamp last, so that it is taken as late
> as possible.
> 2804 */
> 2805 resetStringInfo(&tmpbuf);
> 2806 pq_sendint64(&tmpbuf, GetCurrentTimestamp());
> 2807 memcpy(&output_message.data[1 + sizeof(int64) + sizeof(int64)],
> 2808 tmpbuf.data, sizeof(int64));
> 2809
> 2810 pq_putmessage_noblock('d', output_message.data, output_message.len);
>
>
> After a quick search, I found that WalSndPrepareWrite and WalSndWriteData are always pairs [1].
> IIUC the space of sendtime leave by WalSndPrepareWrite, it always fill out by WalSndWriteData.
>
>
> WalSndWriteData() also fills the timestamp there but it may not always
> be used with WalSndPrepareWrite, at least theoretically. So it's the
> XLogSendPhysical() that it's referring to.

The two functions are the body of two logical-decoding API
functions. They are assumed to be called in that order. See
OutputPluginWrite() for the restriction. The sequence of the two
logica-decoding funcitons and the code block in XLogSendPhysical are
parallels in (theoretically) different protocols.

> Maybe a better way is to separate those two codeblocks into functions
> and use it in WalSndPrepareWrite, WalSndWriteData and XLogSendPhysical
> appropriately. That way we don't have to add a comment like that and
> the sanity of 'w' message is preserved.
>
> Sure, we can write a separate function to fill out the sendtime. Any thoughts?

So I put -1 since they (physical and logical, not prepare and write)
are for distinct protocols.

> [1]
> src/backend/replication/walsender.c:247:static void WalSndPrepareWrite(LogicalDecodingContext *ctx, XLogRecPtr lsn, TransactionId xid, bool last_write);
> src/backend/replication/walsender.c:1015: WalSndPrepareWrite, WalSndWriteData,
> src/backend/replication/walsender.c:1176: WalSndPrepareWrite, WalSndWriteData,
> src/backend/replication/walsender.c:1233:WalSndPrepareWrite(LogicalDecodingContext *ctx, XLogRecPtr lsn, TransactionId xid, bool last_write)
> src/backend/replication/walsender.c:1255: * Actually write out data previously prepared by WalSndPrepareWrite out to

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center