On Thu, Oct 8, 2020 at 7:46 AM Masahiko Sawada
<masahiko.sawada@2ndquadrant.com> wrote:
>
> On Wed, 7 Oct 2020 at 17:52, Amit Kapila <amit.kapila16@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.
