Re: Report bytes and transactions actually sent downtream

Hi All,

On Tue, Nov 18, 2025 at 4:14 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Nov 18, 2025 at 4:05 PM Ashutosh Bapat
> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> >
> > On Tue, Nov 18, 2025 at 3:24 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Tue, Nov 4, 2025 at 4:29 PM Ashutosh Bapat
> > > <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> > > >
> > > > 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?
> >
>
> Sounds reasonable to me.

Sorry for the delayed response. PFA the patch implementing the idea
discussed above. It relies on the output plugin callback to return
correct boolean but maintains the statistics in the core itself.

I have reviewed all the previous comments and applied the ones which
are relevant to the new approach again. Following two are worth noting
here.

In order to address Amit's concern [1] that an inaccuracy in these
counts because of a bug in output plugin code may be blamed on the
core, I have added a note in the documentation of view
pg_stat_replication_slot in order to avoid such a blame and also
directing users to plugin they should investigate.

With the statistics being maintained by the core, Bertrand's concern
about stale statistics [2] are also addressed. Also it does not have
the asymmetry mentioned in point 2 in [3].

Please review.

[1] https://www.postgresql.org/message-id/CAA4eK1KzYaq9dcaa20Pv44ewomUPj_PbbeLfEnvzuXYMZtNw0A%40mail.gmail.com
[2] https://www.postgresql.org/message-id/aNZ1T5vYC1BtKs4M@ip-10-97-1-34.eu-west-3.compute.internal
[3] https://www.postgresql.org/message-id/CAExHW5tfVHABuv1moL_shp7oPrWmg8ha7T8CqwZxiMrKror7iw%40mail.gmail.com

--
Best Wishes,
Ashutosh Bapat