Re: Report bytes and transactions actually sent downtream

On Tue, Sep 23, 2025 at 6:28 PM Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
>
> > 0001 is the previous patch
> > 0002 changes addressing your and Bertrand's comments.
> >
>
> @@ -1573,6 +1573,13 @@ WalSndWriteData(LogicalDecodingContext *ctx,
> XLogRecPtr lsn, TransactionId xid,
> /* output previously gathered data in a CopyData packet */
> pq_putmessage_noblock(PqMsg_CopyData, ctx->out->data, ctx->out->len);
>
> + /*
> + * If output plugin maintains statistics, update the amount of data sent
> + * downstream.
> + */
> + if (ctx->stats)
> + ctx->stats->sentBytes += ctx->out->len + 1; /* +1 for the 'd' */
> +
>
> Just a small observation: I think it’s actually pq_flush_if_writable()
> that writes the buffered data to the socket, not pq_putmessage_noblock
> (which is actually gathering data in the buffer and not sending). So
> it might make more sense to increment the sent pointer after the call
> to pq_flush_if_writable().

That's a good point. I placed it after pq_putmessage_noblock() so that
it's easy to link the increment to sentBytes and the actual bytes
being sent. You are right that the bytes won't be sent unless
pq_flush_if_writable() is called but it will be called for sure before
the next UpdateDecodingStats(). So the reported bytes are never wrong.
I would prefer readability over seeming accuracy.

>
> Should we also consider - pg_hton32((uint32) (len + 4)); -- the
> additional 4 bytes of data added to the send buffer.
>

In WalSndWriteData() we can't rely on what happens in a low level API
like socket_putmessage(). And we are counting the number of bytes in
the logically decoded message. So, I actually wonder whether we should
count 1 byte of 'd' in sentBytes. Shveta, Bertand, what do you think?

--
Best Wishes,
Ashutosh Bapat