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

From Ashutosh Bapat
Subject Re: Report bytes and transactions actually sent downtream
Date
Msg-id CAExHW5vnRPp5dof35DEGdc8W2tS16E5CbmGey28Y=RJ9QrpfAQ@mail.gmail.com
Whole thread Raw
In response to Re: Report bytes and transactions actually sent downtream  (shveta malik <shveta.malik@gmail.com>)
List pgsql-hackers
On Fri, Sep 19, 2025 at 11:48 AM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Thu, Sep 18, 2025 at 3:54 PM Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
> >
> >
> > >
> > > Few comments:
> > >
> > > 1)
> > > postgres=# select slot_name,
> > > total_bytes,plugin_filtered_bytes,plugin_sent_bytes  from
> > > pg_stat_replication_slots order by slot_name;
> > >  slot_name | total_bytes | plugin_filtered_bytes | plugin_sent_bytes
> > > -----------+-------------+-----------------------+-------------------
> > >  slot1     |      800636 |                793188 |               211
> > >  sub1      |      401496 |                132712 |             84041
> > >  sub2      |      401496 |                396184 |               674
> > >  sub3      |      401496 |                145912 |             79959
> > > (4 rows)
> > >
> > > Currently it looks quite confusing. 'total_bytes' gives a sense that
> > > it has to be a sum of filtered and sent. But they are no way like
> > > that. In the thread earlier there was a proposal to change the name to
> > > reordered_txns, reordered_bytes. That looks better to me. It will give
> > > clarity without even someone digging into docs.
> >
> > I also agree with that. But that will break backward compatibility.
>
> Yes, that it will do.
>
> > Do
> > you think other columns like spill_* and stream_* should also be
> > renamed with the prefix "reordered"?
> >
>
> Okay, I see that all fields in pg_stat_replication_slots are related
> to the ReorderBuffer. On reconsideration, I’m unsure whether it's
> appropriate to prefix all of them with reorderd_. For example,
> renaming spill_bytes and stream_bytes to reordered_spill_bytes and
> reordered_stream_bytes. These names start to feel overly long, and I
> also noticed that ReorderBuffer isn’t clearly defined anywhere in the
> documentation (or at least I couldn’t find it), even though the term
> 'reorder buffer' does appear in a few places.
>
> As an example, see ReorderBufferRead, ReorderBufferWrite  wait-types
> at [1]. Also in plugin-doc [2], we use 'ReorderBufferTXN'. And now, we
> are adding: ReorderBufferChangeSize, ReorderBufferChange
>
> This gives me a feeling, will it be better to let
> pg_stat_replication_slots as is and add a brief ReorderBuffer section
> under Logical Decoding concepts [3] just before Output Plugins. And
> then, pg_stat_replication_slots can refer to that section, clarifying
> that the bytes, counts, and txn fields pertain to ReorderBuffer
> (without changing any of the fields).
>
> And then to define plugin related data, we can have a new view, say
> pg_stat_plugin_stats (as Amit suggested earlier) or
> pg_stat_replication_plugins. I understand that adding a new view might
> not be desirable, but it provides better clarity without requiring
> changes to the existing fields in pg_stat_replication_slots. I also
> strongly feel that to properly tie all this information together, a
> brief definition of the ReorderBuffer is needed. Other pages that
> reference this term can then point to that section. Thoughts?

Even if we keep two views, when they are joined, users will still get
confused by total_* names. So it's not solving the underlying problem.
Andres had raised the point about renaming total_* fields with me
off-list earlier. He suggested names total_wal_bytes, and
total_wal_txns in an off list discussion today. I think those convey
the true meaning - that these are txns and bytes that come from WAL.
Used those in the attached patches. Prefix reordered would give away
lower level details, so I didn't use it.

I agree that it would be good to mention ReorderBuffer in the logical
decoding concepts section since it mentions structures ReorderBuffer*.
But that would be a separate patch since we aren't using "reordered"
in the names of the fields.

0001 is the previous patch
0002 changes addressing your and Bertrand's comments.

--
Best Wishes,
Ashutosh Bapat

Attachment

pgsql-hackers by date:

Previous
From: "Matheus Alcantara"
Date:
Subject: Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue
Next
From: Ashutosh Bapat
Date:
Subject: Re: POC: enable logical decoding when wal_level = 'replica' without a server restart