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. -- With Regards, Amit Kapila.
v2-0001-Track-streaming-stats.patch
Description: Binary data
