Hi, thank you for the awesome feature. As it may have been discussed, I think the 'name' column in pg_stat_replication_slots is more consistent with the column name and data type matched to the pg_replication_slots catalog. The attached patch changes the name and data type of the 'name' column to slot_name and 'name' type, respectively. Also, the macro name PG_STAT_GET_..._CLOS has been changed to PG_STAT_GET_..._COLS.
Regards, Noriyoshi Shinoda -----Original Message----- From: Masahiko Sawada [mailto:masahiko.saw...@2ndquadrant.com] Sent: Monday, October 12, 2020 3:22 PM To: Amit Kapila <amit.kapil...@gmail.com> Cc: Dilip Kumar <dilipbal...@gmail.com>; Magnus Hagander <mag...@hagander.net>; Tomas Vondra <tomas.von...@2ndquadrant.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Ajin Cherian <itsa...@gmail.com> Subject: Re: Resetting spilled txn statistics in pg_stat_replication On Thu, 8 Oct 2020 at 22:57, Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Oct 8, 2020 at 7:46 AM Masahiko Sawada > <masahiko.saw...@2ndquadrant.com> wrote: > > > > On Wed, 7 Oct 2020 at 17:52, Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > > > I think after we are done with this the next step would be to > > > finish the streaming stats work [1]. We probably need to review > > > and add the test case in that patch. If nobody else shows up I > > > will pick it up and complete it. > > > > +1 > > I can review that patch. > > > > I have rebased the stream stats patch and made minor modifications. I > haven't done a detailed review but one thing that I think is not > correct is: > @@ -3496,10 +3499,18 @@ ReorderBufferStreamTXN(ReorderBuffer *rb, > ReorderBufferTXN *txn) > txn->snapshot_now = NULL; > } > > + > + rb->streamCount += 1; > + rb->streamBytes += txn->total_size; > + > + /* Don't consider already streamed transaction. */ > + rb->streamTxns += (rbtxn_is_streamed(txn)) ? 0 : 1; > + > /* Process and send the changes to output plugin. */ > ReorderBufferProcessTXN(rb, txn, InvalidXLogRecPtr, snapshot_now, > command_id, true); > > I think we should update the stream stats after > ReorderBufferProcessTXN rather than before because any error in > ReorderBufferProcessTXN can lead to an unnecessary update of stats. > But OTOH, the txn flags, and other data can be changed after > ReorderBufferProcessTXN so we need to save them in a temporary > variable before calling the function. Thank you for updating the patch! I've not looked at the patch in-depth yet but RBTXN_IS_STREAMED could be cleared after ReorderBUfferProcessTXN()? BTW maybe it's better to start a new thread for this patch as the title is no longer relevant. Regards, -- Masahiko Sawada http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
pg_stat_replication_slots.diff
Description: pg_stat_replication_slots.diff
