On Sat, Apr 10, 2021 at 9:50 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Fri, Apr 9, 2021 at 4:13 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > 2. > > @@ -2051,6 +2054,17 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, > > ReorderBufferTXN *txn, > > rb->begin(rb, txn); > > } > > > > + /* > > + * Update total transaction count and total transaction bytes, if > > + * transaction is streamed or spilled it will be updated while the > > + * transaction gets spilled or streamed. > > + */ > > + if (!rb->streamBytes && !rb->spillBytes) > > + { > > + rb->totalTxns++; > > + rb->totalBytes += rb->size; > > + } > > > > I think this will skip a transaction if it is interleaved between a > > streaming transaction. Assume, two transactions t1 and t2. t1 sends > > changes in multiple streams and t2 sends all changes in one go at > > commit time. So, now, if t2 is interleaved between multiple streams > > then I think the above won't count t2. > > > > 3. > > @@ -3524,9 +3538,11 @@ ReorderBufferSerializeTXN(ReorderBuffer *rb, > > ReorderBufferTXN *txn) > > { > > rb->spillCount += 1; > > rb->spillBytes += size; > > + rb->totalBytes += size; > > > > /* don't consider already serialized transactions */ > > rb->spillTxns += (rbtxn_is_serialized(txn) || > > rbtxn_is_serialized_clear(txn)) ? 0 : 1; > > + rb->totalTxns += (rbtxn_is_serialized(txn) || > > rbtxn_is_serialized_clear(txn)) ? 0 : 1; > > } > > > > We do serialize each subtransaction separately. So totalTxns will > > include subtransaction count as well when serialized, otherwise not. > > The description of totalTxns also says that it doesn't include > > subtransactions. So, I think updating rb->totalTxns here is wrong. > > > > The attached patch should fix the above two comments. I think it > should be sufficient if we just update the stats after processing the > TXN. We need to ensure that don't count streamed transactions multiple > times. I have not tested the attached, can you please review/test it > and include it in the next set of patches if you agree with this > change. >
oops, forgot to attach. Attaching now. -- With Regards, Amit Kapila.
fix_stats_1.patch
Description: Binary data