Re: Report bytes and transactions actually sent downtream - Mailing list pgsql-hackers

From Bertrand Drouvot
Subject Re: Report bytes and transactions actually sent downtream
Date
Msg-id aNZ1T5vYC1BtKs4M@ip-10-97-1-34.eu-west-3.compute.internal
Whole thread Raw
In response to Re: Report bytes and transactions actually sent downtream  (Bertrand Drouvot <bertranddrouvot.pg@gmail.com>)
Responses Re: Report bytes and transactions actually sent downtream
List pgsql-hackers
Hi,

On Thu, Sep 25, 2025 at 01:01:55PM +0000, Bertrand Drouvot wrote:
> Hi,
> 
> On Thu, Sep 25, 2025 at 10:16:35AM +0530, Ashutosh Bapat wrote:
> > Do you have any further review comments?
> 
> Not right now. I'll give it another look by early next week the latest.
> 

=== 1

@@ -173,6 +173,7 @@ pg_decode_startup(LogicalDecodingContext *ctx, OutputPluginOptions *opt,
        data->only_local = false;

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

I was not sure where it's allocated, but looking at:

Breakpoint 1, pg_decode_startup (ctx=0x1ba853a0, opt=0x1ba85478, is_init=false) at test_decoding.c:164
164             bool            enable_streaming = false;
(gdb) n
166             data = palloc0(sizeof(TestDecodingData));
(gdb)
167             data->context = AllocSetContextCreate(ctx->context,
(gdb)
170             data->include_xids = true;
(gdb)
171             data->include_timestamp = false;
(gdb)
172             data->skip_empty_xacts = false;
(gdb)
173             data->only_local = false;
(gdb)
175             ctx->output_plugin_private = data;
(gdb)
176             ctx->stats = palloc0(sizeof(OutputPluginStats));
(gdb)
178             opt->output_type = OUTPUT_PLUGIN_TEXTUAL_OUTPUT;
(gdb) p CurrentMemoryContext
$7 = (MemoryContext) 0x1ba852a0
(gdb) p (*CurrentMemoryContext).name
$8 = 0xe4057d "Logical decoding context"
(gdb) p ctx->context
$9 = (MemoryContext) 0x1ba852a0

I can see that CurrentMemoryContext is "ctx->context" so the palloc0 done here
are done in the right context.

=== 2

Playing with "has stats" a bit.

-- Issue 1:

Say, plugin has stats enabled and I get:

postgres=# select plugin,plugin_sent_txns from pg_stat_replication_slots ;
     plugin     | plugin_sent_txns
----------------+------------------
 pg_commit_info |                9
(1 row)

If the engine is shutdown and the plugin is now replaced by a version that
does not provide stats, then, right after startup, I still get:

postgres=# select plugin,plugin_sent_txns from pg_stat_replication_slots ;
     plugin     | plugin_sent_txns
----------------+------------------
 pg_commit_info |                9
(1 row)

And that will be the case until the plugin decodes something (so that 
statent->plugin_has_stats gets replaced in pgstat_report_replslot()).

That's because plugin_has_stats is stored in PgStat_StatReplSlotEntry
and so it's restored from the stat file when the engine starts.

Now, let's do some inserts and decode:

postgres=# insert into t1 values ('a');
INSERT 0 1
postgres=# insert into t1 values ('a');
INSERT 0 1
postgres=# select * from pg_logical_slot_get_changes('logical_slot',NULL,NULL);
    lsn     | xid |                                          data
------------+-----+-----------------------------------------------------------------------------------------
 0/407121C0 | 766 | xid 766: lsn:0/40712190 inserts:1 deletes:0 updates:0 truncates:0 relations truncated:0
 0/40712268 | 767 | xid 767: lsn:0/40712238 inserts:1 deletes:0 updates:0 truncates:0 relations truncated:0
(2 rows)

postgres=# select plugin,plugin_sent_txns from pg_stat_replication_slots ;
     plugin     | plugin_sent_txns
----------------+------------------
 pg_commit_info |
(1 row)

All good. 

Issue 1 is that before any decoding happens, pg_stat_replication_slots is still
showing stale plugin statistics from a plugin that may no longer support stats.

I'm not sure how we could easily fix this issue, as we don't know the plugin's
stats capability until we actually use it.

-- Issue 2:

Let's shutdown, replace the plugin with a version that has stats enabled and
restart.

Same behavior as before:

postgres=# select plugin,plugin_sent_txns from pg_stat_replication_slots ;
     plugin     | plugin_sent_txns
----------------+------------------
 pg_commit_info |
(1 row)

Until pgstat_report_replslot() is not called, the statent->plugin_has_stats is
not updated. So it displays the stats as they were before the shutdown. But that's
not an issue in this case (when switching from non stats to stats).

Now, let's do some inserts and decode:

postgres=# insert into t1 values ('a');
INSERT 0 1
postgres=# select * from pg_logical_slot_get_changes('logical_slot',NULL,NULL);
    lsn     | xid |                                          data
------------+-----+-----------------------------------------------------------------------------------------
 0/407125B0 | 768 | xid 768: lsn:0/40712580 inserts:1 deletes:0 updates:0 truncates:0 relations truncated:0
(1 row)

and check the stats:

postgres=# select plugin,plugin_sent_txns from pg_stat_replication_slots ;
     plugin     | plugin_sent_txns
----------------+------------------
 pg_commit_info |               10
(1 row)

Now it reports 10, that's the 9 before we changed the plugin to not have stats
enabled plus this new one.

Issue 2: when switching from a non-stats plugin back to a stats-capable plugin, it
shows accumulated values from before the non-stats switch.

PFA attached a proposal to fix Issue 2.

Regards,

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

Attachment

pgsql-hackers by date:

Previous
From: Dilip Kumar
Date:
Subject: Re: Proposal: Conflict log history table for Logical Replication
Next
From: David Christensen
Date:
Subject: Re: [PATCH] GROUP BY ALL