| 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
| 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 |