Re: Report bytes and transactions actually sent downtream

On Tue, Jun 30, 2026 at 5:39 AM Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, Jun 30, 2026 at 12:31 PM Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
> >
> > Thank you, Masahiko-san, for your review comments. Please find my
> > responses inline below:
> >
> > On Tue, Jun 30, 2026 at 4:32 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > >
> > > ---
> > > @@ -1979,6 +1980,7 @@ UpdateDecodingStats(LogicalDecodingContext *ctx)
> > > repSlotStat.mem_exceeded_count = rb->memExceededCount;
> > > repSlotStat.total_txns = rb->totalTxns;
> > > repSlotStat.total_bytes = rb->totalBytes;
> > > + repSlotStat.output_bytes = rb->outputBytes;
> > >
> > > Since output_bytes is now the tenth field we accumulate for
> > > replication slot statistics, it seems like a good opportunity to
> > > improve its maintainability. Instead of adding fields individually, we
> > > could define a common structure for these shared metrics so that both
> > > PgStat_StatReplSlotEntry and ReorderBuffer can embed it. This way,
> > > UpdateDecodingStats() could simply assign the collected struct and
> > > clear it using MemSet().
> > >
> >
> > I agree that this would improve maintainability.
> > PgStat_StatReplSlotEntry now has 13 fields, and 10 of them are the
> > decoding counters that are also tracked in ReorderBuffer, so a common
> > counter-only structure sounds like the right direction.
> >
> > That said, this is not specific to the output_bytes field being added
> > here. It is more of a refactoring of the boundary between logical
> > decoding and pgstat statistics. To keep this patch focused, I think it
> > would be better to handle that as a separate follow-up patch, possibly
> > with a new thread once this work is done. Please let me know your
> > thoughts on this?
> >
>
> In case you'd prefer to have the refactoring done as part of this
> effort, I'm attaching a patch that performs the refactoring described
> above, with the patch for reporting output_bytes built on top of it.
> Please have a look and let me know your feedback.

Thank you for updating the patch.

I agree that the refactoring part should be done in a separate patch.

Could you swap these patches' order? I think that these patches are
independent of each other and while we have an agreement on the new
statistics, we still need discussion for the refactoring part
(particularly I guess it's better to avoid including pgstatt.h in
reorderbuffer.h)

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com