Re: Report bytes and transactions actually sent downtream

On Mon, Nov 3, 2025 at 8:50 PM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2025-11-03 19:53:30 +0530, Ashutosh Bapat wrote:
> > This commit adds following fields to pg_stat_replication_slots
> > - plugin_filtered_bytes is the amount of changes filtered out by the
> >   output plugin
> > - plugin_sent_txns is the amount of transactions sent downstream by the
> >   output plugin
> > - plugin_sent_bytes is the amount of data sent downstream by the output
> >   plugin.
> >
> > The prefix "plugin_" indicates that these counters are related to and
> > maintained by the output plugin. An output plugin may choose not to
> > initialize LogicalDecodingContext::stats, which holds these counters, in
> > which case the above columns will be reported as NULL.
>
> I continue to be uncomfortable with doing all this tracking explicitly in
> output plugins. This still seems like something core infrastructure should
> take care of, instead of re-implementing it in different output plugins, with
> the inevitable behaviour differences that will entail.

I understand your concern, and while I agree that it's ideal to keep
as much of the stats bookkeeping in core there are some nuances here
which makes it hard as explained below.

My first patch [1] had the stats placed in ReorderBuffer directly. It
was evident from the patch that the sentTxns needs to be set somewhere
in the output plugin code since the output plugin may decide to filter
out or send transaction when processing a change in that transaction
(not necessarily when in begin_cb). Filtered bytes is also something
that is in plugin's control and needs to be updated in the output
plugin code. Few emails, starting from [2], discussed possible
approaches to maintain those in the core vs maintain those in the
output plugin. We decided to let output plugin maintain it for
following reasons

a. sentTxns and filteredBytes need to be modified in the output plugin
code. The behaviour there is inherently output plugin specific, and
requires output plugin specific implementation.
b. an output plugin may or may not want to update their code to track
the statistics for various logistic and technical reasons. We need to
be flexible about that if possible.

The current approach requires only the output plugin specific changes
to be made to the output plugin code and also makes it optional for
them to do those changes. The only changes in output plugin code are
for a. indicating whether it updates the stats and b. updating
filteredBytes and sentBytes at appropriate places. I don't see a way
to avoid that. Rest of the logic is actually in the core. Unless
there's anything we've overlooked in the thread the current approach
seems to balance the constraints quite well. Do you have an
alternative design in mind?

This has been a long thread with many patch versions, and the commit
message might need some rewording to describe the proposed
functionality better.  I hope the above explanation is clearer, and if
so I can reword the commit message to include more of it.

sentBytes is a slightly different story. The core code updates it. But
it's a stat about output plugin's behaviour. Hence it's still exposed
as a plugin stats and maintained in LogicalDecodingContext::stats. It
can be maintained in ReorderBuffer directly and can be projected as a
core stats, renaming it as just "sent_bytes". Please let me know if
you would like it that way.

[1] https://www.postgresql.org/message-id/CAExHW5s6KntzUyUoMbKR5dgwRmdV2Ay_2%2BAnTgYGAzo%3DQv61wA%40mail.gmail.com
[2] https://www.postgresql.org/message-id/CAA4eK1KzYaq9dcaa20Pv44ewomUPj_PbbeLfEnvzuXYMZtNw0A%40mail.gmail.com

--
Best Wishes,
Ashutosh Bapat