Hi,
On Tue, Jun 30, 2026 at 12:31 PM Ashutosh Sharma <ashu.coek88@gmail.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.mshk@gmail.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.
--
With Regards,
Ashutosh Sharma.
