Re: Resetting spilled txn statistics in pg_stat_replication - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: Resetting spilled txn statistics in pg_stat_replication
Date
Msg-id CAA4eK1++=2XDNqqd=KT5yp51kJjOACv-XEYLtqXPGiwNPFPK1A@mail.gmail.com
Whole thread Raw
In response to Re: Resetting spilled txn statistics in pg_stat_replication  (Masahiko Sawada <masahiko.sawada@2ndquadrant.com>)
List pgsql-hackers
On Mon, Oct 12, 2020 at 11:52 AM Masahiko Sawada
<masahiko.sawada@2ndquadrant.com> wrote:
>
> On Thu, 8 Oct 2020 at 22:57, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Thu, Oct 8, 2020 at 7:46 AM Masahiko Sawada
> > <masahiko.sawada@2ndquadrant.com> wrote:
> >
> > 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()?
>

I think you mean to say RBTXN_IS_STREAMED could be *set* after
ReorderBUfferProcessTXN(). We need to set it for txn and subtxns and
currently, it is being done with other things in
ReorderBufferTruncateTXN so not sure if it is a good idea to do this
separately.

> BTW maybe it's better to start a new thread for this patch as the
> title is no longer relevant.
>

Yeah, that makes sense. I'll do that while posting a new version of the patch.


-- 
With Regards,
Amit Kapila.



pgsql-hackers by date:

Previous
From: Heikki Linnakangas
Date:
Subject: Re: partition routing layering in nodeModifyTable.c
Next
From: Michael Paquier
Date:
Subject: Re: speed up unicode normalization quick check