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

On Wed, Oct 29, 2025 at 9:14 AM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Tue, Oct 28, 2025 at 12:46 PM Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
> >
> > On Mon, Oct 27, 2025 at 4:47 PM shveta malik <shveta.malik@gmail.com> wrote:
> > >
> > > Few comments:
> > >
> > > 1)
> > > pgoutput_truncate:
> > >
> > > if (nrelids > 0)
> > > {
> > > OutputPluginPrepareWrite(ctx, true);
> > > logicalrep_write_truncate(ctx->out,
> > >   xid,
> > >   nrelids,
> > >   relids,
> > >   change->data.truncate.cascade,
> > >   change->data.truncate.restart_seqs);
> > > OutputPluginWrite(ctx, true);
> > > }
> > > + else
> > > + ctx->stats->filteredBytes += ReorderBufferChangeSize(change);
> > > +
> > >
> > > It seems that filteredBytes are only counted for TRUNCATE when nrelids
> > > is 0. Can nrelids only be 0 or same as nrelations?
> > >
> > > The below code makes me think that nrelids can be any number between 0
> > > and nrelations, depending on which relations are publishable and which
> > > supports publishing TRUNCATE. If that’s true, shouldn’t we count
> > > filteredBytes in each such skipped case?
> >
> > IIIUC, you are suggesting that we should add
> > ReorderBufferChangeSize(change) for every relation which is not part
> > of the publication or whose truncate is not published.
>
> No, that will be wrong.
>
> > I think that
> > won't be correct since it can lead to a situation where filtered bytes
> > > total bytes which should never happen. Even if there is a single
> > publishable relation whose truncate is published, the change should
> > not be considered as filtered since something would be output
> > downstream.
>
> Yes, the entire change should not be treated as filtered. The idea is
> that, for example, if there are 20 relations belonging to different
> publications and only one of them supports publishing TRUNCATE, then
> when a TRUNCATE is triggered on all, the data for that one relation
> should be counted as sent (which is currently happening based on
> nrelids), while the data for the remaining 19 should be considered
> filtered — which is not happening right now.
>
> > Otherwise filtered bytes as well as sent bytes both will
> > be incremented causing an inconsistency (which would be hard to notice
> > since total bytes - filtered bytes has something to do with the sent
> > bytes but the exact correlation is hard to grasp in a formula).
> >
> > We may increment filteredBytes by sizeof(OID) for every relation we
> > skip here OR by ReoderBufferChangeSize(change) if all the relations
> > are filtered, but that's too much dependent on how the WAL record is
> > encoded; and adding that dependency in an output plugin code seems
> > hard to manage.
> >
>
> Yes, that was the idea, to increment filteredBytes in this way. But I
> see your point. I can’t think of a better solution at the moment. If
> you also don’t have any better ideas, then at least adding a comment
> in this function would be helpful. Right now, it looks like we
> overlooked the fact that some relationships should contribute to
> filteredBytes while others should go to sentBytes.

I noticed that we do something similar while filtering columns. I
think we need to add a comment in that code as well. How about
something like below?

diff --git a/src/backend/replication/pgoutput/pgoutput.c
b/src/backend/replication/pgoutput/pgoutput.c
index 4b35f2de6aa..f2d6e20a702 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -1621,7 +1621,12 @@ pgoutput_change(LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,

        OutputPluginPrepareWrite(ctx, true);

-       /* Send the data */
+       /*
+        * Send the data. Even if we end up filtering some columns
while sending the
+        * message, we won't consider the change, as a whole, to be
filtered out.
+        * Instead the filtered columns will be reflected as a smaller sentBytes
+        * count.
+        */
        switch (action)
        {
                case REORDER_BUFFER_CHANGE_INSERT:
@@ -1728,6 +1733,13 @@ pgoutput_truncate(LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,

change->data.truncate.cascade,

change->data.truncate.restart_seqs);
                OutputPluginWrite(ctx, true);
+
+               /*
+                * Even if we filtered out some relations, we still
send a TRUNCATE
+                * message for the remaining relations. Since the
change, as a whole, is
+                * not filtered out, we don't count modify
filteredBytes. The filtered
+                * out relations will be reflected as a smaller sentBytes count.
+                */
        }
        else
                ctx->stats->filteredBytes += ReorderBufferChangeSize(change);

--
Best Wishes,
Ashutosh Bapat