Re: Report bytes and transactions actually sent downtream

From: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Report bytes and transactions actually sent downtream
Date: 2025-08-27 13:43:49
Message-ID: aK8LlVEAKLGsIytg@ip-10-97-1-34.eu-west-3.compute.internal
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Dean Rasheed 2025-08-27 13:44:55 Re: Inconsistent update in the MERGE command
Previous Message Melanie Plageman 2025-08-27 13:08:28 Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)