Re: Report bytes and transactions actually sent downtream

Hi,

On Wed, Sep 24, 2025 at 05:28:44PM +0530, Ashutosh Bapat wrote:
> On Wed, Sep 24, 2025 at 2:38 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > On Wed, Sep 24, 2025 at 12:47 PM Ashutosh Bapat
> > <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> > >
> > > On Wed, Sep 24, 2025 at 10:12 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> > > >
> > > > I tested the flows with
> > > > a) logical replication slot and get-changes.
> > > > b) filtered data flows: pub-sub creation with row_filters, 'publish'
> > > > options. I tried to verify plugin fields as compared to total_wal*
> > > > fields.
> > > > c) reset flow.
> > > >
> > > > While tests for a and c are present already. I don't see tests for b
> > > > anywhere when it comes to stats. Do you think we shall add a test for
> > > > filtered data using row-filter somewhere?
> > >
> > > Added a test in 028_row_filter. Please find it in the attached
> > > patchset.
> >
> > Test looks good.
>
> Thanks. Added to three more files. I think we have covered all the
> cases where filtering can occur.
>
> PFA patches.

Thanks for the new version!

A few random comments:

=== 1

+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>plugin_filtered_bytes</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Amount of changes, from <structfield>total_wal_bytes</structfield>, filtered
+ out by the output plugin and not sent downstream. Please note that it
+ does not include the changes filtered before a change is sent to
+ the output plugin, e.g. the changes filtered by origin. The count is
+ maintained by the output plugin mentioned in
+ <structfield>plugin</structfield>.

I found "The count" somehow ambiguous. What about "This statistic" instead?

=== 2

+ subtransactions. These transactions are subset of transctions sent to

s/transctions/transactions

=== 3

+ the decoding plugin. Hence this count is expected to be lesser than or

s/be lesser/be less/? (not 100% sure)

=== 4

+extern Size ReorderBufferChangeSize(ReorderBufferChange *change);

Another approach could be to pass the change's size as an argument to the
callbacks? That would avoid to expose ReorderBufferChangeSize publicly.

=== 5

ctx->output_plugin_private = data;
+ ctx->stats = palloc0(sizeof(OutputPluginStats));

I was wondering if we need to free this in pg_decode_shutdown, but it looks
like it's done through FreeDecodingContext() anyway.

Regards,

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