On Wed, Sep 24, 2025 at 11:08 AM Ashutosh Bapat <[email protected]> wrote: > > On Tue, Sep 23, 2025 at 6:28 PM Ashutosh Sharma <[email protected]> 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
