Re: Fix typo about WalSndPrepareWrite

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.

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?

[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

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.