Re: Report bytes and transactions actually sent downtream

Hi,

On Thu, Jul 24, 2025 at 12:24:26PM +0530, Ashutosh Bapat wrote:
> Here's the next patch which considers all the discussion so far. It
> adds four fields to pg_stat_replication_slots.
>     - plugin - name of the output plugin

Is this one needed? (we could get it with a join on pg_replication_slots)

>     - plugin_filtered_bytes - reports the amount of changes filtered
> out by the output plugin
>     - plugin_sent_txns - the amount of transactions sent downstream by
> the output plugin
>     - plugin_sent_bytes - the amount of data sent downstream by the
> outputplugin.
> 
> There are some points up for a discussion:
> 1. pg_stat_reset_replication_slot() zeroes out the statistics entry by
> calling pgstat_reset() or pgstat_reset_of_kind() which don't know
> about the contents of the entry. So
> PgStat_StatReplSlotEntry::plugin_has_stats is set to false and plugin
> stats are reported as NULL, instead of zero, immediately after reset.
> This is the same case when the stats is queried immediately after the
> statistics is initialized and before any stats are reported. We could
> instead make it report
> zero, if we save the plugin_has_stats and restore it after reset. But
> doing that in pgstat_reset_of_kind() seems like an extra overhead + we
> will need to write a function to find all replication slot entries.

Could we store plugin_has_stats in ReplicationSlotPersistentData instead? That
way it would not be reset. We would need to access ReplicationSlotPersistentData
in pg_stat_get_replication_slot though.

Also would that make sense to expose plugin_has_stats in pg_replication_slots?

> 2. There's also a bit of asymmetry in the way sent_bytes is handled.
> The code which actually sends the logical changes to the downstream is
> part of the core code
> but the format of the change and hence the number of bytes sent is
> decided by the plugin. It's a stat related to plugin but maintained by
> the core code. The patch implements it as a plugin stat (so the
> corresponding column has "plugin" prefix

The way it is done makes sense to me.
 
> 3. The names of new columns have the prefix "plugin_" but the internal
> variables tracking those don't for the sake of brevity. If you prefer
> to have the same prefix for the internal variables, I can change that.

Just my taste: I do prefer when they match.

Regards,

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