Re: Report bytes and transactions actually sent downtream - Mailing list pgsql-hackers

On Tue, Nov 18, 2025 at 3:24 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, Nov 4, 2025 at 4:29 PM Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
> >
> > 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.
> >
>
> Is it possible that we allow change callback (LogicalDecodeChangeCB)
> to return a boolean such that if the change is decoded and sent, it
> returns true, otherwise, false? If so, the caller could deduce from it
> the filtered bytes, and if none of the change calls returns true, this
> means the entire transaction is not sent.
>
> I think this should address Andres's concern of explicitly tracking
> these stats in plugins, what do you think?
>

I was thinking about a similar thing. But I am skeptical since the
calling logic is not straight forward - there's an indirection in
between. Second, it means that all the plugins have to adapt to the
new callback definition. It is optional in my current approach. Since
both of us have thought of this approach, I think it's worth a try.

"if none of the change calls returns true, this means the entire
transaction is not sent" isn't true. A plugin may still send an empty
transaction. I was thinking of making commit/abort/prepare callbacks
to return true/false to indicate whether a transaction was sent or not
and increment the counter accordingly. The plugin has to take care of
not returning true for both prepare and commit or prepare and abort.
So may be just commit and abort should be made to return true or
false. What do you think?

--
Best Wishes,
Ashutosh Bapat