On Tue, Jun 30, 2026 at 5:39 AM Ashutosh Sharma <[email protected]> wrote: > > Hi, > > On Tue, Jun 30, 2026 at 12:31 PM Ashutosh Sharma <[email protected]> > 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 <[email protected]> > > 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
