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>, Andres Freund <andres(at)anarazel(dot)de>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Report bytes and transactions actually sent downtream
Date: 2025-12-18 18:22:48
Message-ID: aURGeIVgkWOUBipK@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, Dec 18, 2025 at 06:22:40PM +0530, Ashutosh Bapat wrote:
> Hi Bertrand,
>
> On Wed, Dec 17, 2025 at 2:12 PM Bertrand Drouvot
> <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
> >
> > What worries me is all those API changes:
> >
> > -typedef void (*LogicalDecodeChangeCB) (struct LogicalDecodingContext *ctx,
> > +typedef bool (*LogicalDecodeChangeCB) (struct LogicalDecodingContext *ctx,
> >
> > Those changes will break existing third party logical decoding plugin, even ones
> > that don't want the new statistics features.
> >
> > What about not changing those and just add a single new optional callback, say?
> >
> > typedef void (*LogicalDecodeReportStatsCB)(
> > LogicalDecodingContext *ctx,
> > ReorderBufferTXN *txn,
> > bool *transaction_sent,
> > size_t *bytes_filtered
> > );
> >
> > This way:
> >
> > - Existing plugins can still work without modification
> > - New or existing plugins can choose to provide statistics
> >
>
> I think that it will bring back the same problems that the previous
> design had or am I missing something?

I think that my example was confusing due to "size_t *bytes_filtered". I think
that what we could do is something like:

"
typedef void (*LogicalDecodeReportStatsCB)(
LogicalDecodingContext *ctx,
LogicalDecodeEventType event_type,
bool *filtered,
bool *txn_sent);
"

Note that there is no more size_t.

Then for, for example in change_cb_wrapper(), we could do:

"
ctx->callbacks.change_cb(ctx, txn, relation, change);

if (ctx->callbacks.report_stats_cb)
{
bool filtered = false;

ctx->callbacks.report_stats_cb(ctx, LOGICALDECODE_CHANGE,
&filtered, NULL);

if (filtered)
cache->filteredBytes += ReorderBufferChangeSize(change);
}
"

The plugin would need to "remember" that it filtered (so that it can
reply to the callback). It could do that by adding say "last_event_filtered" to
it's output_plugin_private structure.

That's more work on the plugin side and we would probably need to provide some
examples from our side.

I think the pros are that:

- plugins that don't want to report stats would have nothing to do (no breaking
changes)
- the core does the computation

Thoughts?

Regards,

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Zsolt Parragi 2025-12-18 18:28:11 Re: Custom oauth validator options
Previous Message Jacob Champion 2025-12-18 18:20:21 Re: Serverside SNI support in libpq