On Wed, Sep 24, 2025 at 11:08 AM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> On Tue, Sep 23, 2025 at 6:28 PM Ashutosh Sharma <ashu.coek88@gmail.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?
>
If we are not counting all such metadata bytes ((or can't reliably do
so), then IMO, we shall skip counting msgtype as well.
thanks
Shveta
