Re: Report bytes and transactions actually sent downtream

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>
Cc: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Report bytes and transactions actually sent downtream
Date: 2026-03-16 23:21:31
Message-ID: abiQe4fxR2fp317F@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Mar 16, 2026 at 01:20:57PM +0530, Ashutosh Sharma wrote:
> 1) v20260316-0001-Report-downstream-sent-bytes-in-pg_stat_replication_.patch:
>
> This patch introduces sent_bytes to report the amount of data sent
> downstream. It documents sent_bytes in a way that it clarifies how
> sent_bytes differs from total_bytes, without modifying the existing
> documentation for total_bytes or total_txns. The patch is purely
> additive and does not alter any existing documentation.

+ elog(DEBUG2, "UpdateDecodingStats: updating stats %p %" PRId64 "
%" PRId64 " %" PRId64 " %" PRId64 " %" PRId64 " %" PRId64 " %" PRId64
" %" PRId64 " %" PRId64 " %" PRId64,

This was already unreadable before the patch, and it makes the
situation worse. Could it be possible to append the names of each
field here?

+ qq(Check that reset timestamp is later after the second reset of
stats for slot '$stats_test_slot1' and confirm total_bytes and
sent_bytes were set to 0.)

Not your fault here as well, but I would replace that by a comment at
the top of the test. Still your patch makes things harder to read.

+ Amount of transaction changes sent downstream for this slot by the
+ output plugin after applying output plugin filters, if any, and
+ converting it into the output plugin format.

Do we need to be that fancy here? The concept relates to the number
of bytes sent downstream. Here is a suggestion:
"Amount of bytes decoded and sent downstream by the output
plugin. This accounts for the output plugin filters, if any, and for
the conversion into the output plugin format."

Perhaps we had better document at the top of LogicalOutputWrite() that
"sentBytes" ought to be updated for all the fields we are sending part
of the tuplestore?

Why are you not counting the "now" timestamp in WalSndWriteData()?
This is appended to the stream of data with a separate pq_send*()
call. Perhaps the reason why this is not appended is worth a comment?

The trick with this patch is to make sure that all the relevant places
where data is sent downstream are correctly incremented. As far as I
can see, things seem to be covered, but I cannot help but wonder if we
are missing one or more places. @Amit, do you feel a hole somewhere?
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Lukas Fittl 2026-03-16 23:25:05 Re: pg_plan_advice
Previous Message Fujii Masao 2026-03-16 23:16:08 Re: Shutdown indefinitely stuck due to unflushed FPI_FOR_HINT record