Re: Report bytes and transactions actually sent downtream

Hi,

On Fri, Sep 26, 2025 at 06:14:28PM +0530, Ashutosh Bapat wrote:
> On Fri, Sep 26, 2025 at 4:43 PM Bertrand Drouvot
> <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
> >
> >
> > === 2
> >
> > Issue 1 is that before any decoding happens, pg_stat_replication_slots is still
> > showing stale plugin statistics from a plugin that may no longer support stats.
> >
> > I'm not sure how we could easily fix this issue, as we don't know the plugin's
> > stats capability until we actually use it.
> >
>
> I don't think this is an issue. There is no way for the core to tell
> whether the plugin will provide stats or not, unless it sets that
> ctx->stats which happens in the startup callback. Till then it is
> rightly providing the values accumulated so far. Once the decoding
> starts, we know that the plugin is not providing any stats and we
> don't display anything.

Yeah, I got the technical reasons, but I think there's a valid user experience
concern here: seeing statistics for a plugin that doesn't actually support
statistics is misleading.

What we need is a call to pgstat_report_replslot() to display stats that reflect
the current plugin behavior. We can't just call pgstat_report_replslot()
in say RestoreSlotFromDisk() because we really need the decoding to start.

So one idea could be to set a flag (per slot) when pgstat_report_replslot()
has been called (for good reasons) and check for this flag in
pg_stat_get_replication_slot().

If the flag is not set, then set the plugin fields to NULL.
If the flag is set, then display their values (like now).

And we should document that the plugin stats are not available (i.e are NULL)
until the decoding has valid stats to report after startup.

What do you think?

>
> > -- Issue 2:
> >
> > Now it reports 10, that's the 9 before we changed the plugin to not have stats
> > enabled plus this new one.
> >
> > Issue 2: when switching from a non-stats plugin back to a stats-capable plugin, it
> > shows accumulated values from before the non-stats switch.
>
> This too seems to be a non-issue to me. The stats in the view get
> reset only when a user resets them. So we shouldn't wipe out the
> already accumulated values just because the plugin stopped providing
> it. If the plugin keeps flip-flopping and only partial statistics
> provided by the plugin will be accumulated. That's the plugin's
> responsibility.

Okay but then I think that the plugin is missing some flexibility.

For example, how could the plugin set ctx->stats->sentTxns
to zero if it decides not to enable stats (while it was previously enable)?

Indeed, not enabling stats, means not doing
"ctx->stats = palloc0(sizeof(OutputPluginStats))" which means not having control
over the stats anymore.

So, with the current design, it has not other choice but having its previous
stats not reset to zero.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com