On Mon, Oct 21, 2019 at 2:50 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Oct 21, 2019 at 10:48 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > On Fri, Oct 18, 2019 at 5:32 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > 3. > > > @@ -2479,7 +2480,7 @@ ReorderBufferSerializeTXN(ReorderBuffer *rb, > > > ReorderBufferTXN *txn) > > > > > > /* update the statistics */ > > > rb->spillCount += 1; > > > - rb->spillTxns += txn->serialized ? 1 : 0; > > > + rb->spillTxns += txn->serialized ? 0 : 1; > > > rb->spillBytes += size; > > > > > > Why is this change required? Shouldn't we increase the spillTxns > > > count only when the txn is serialized? > > > > Prior to this change it was increasing the rb->spillTxns, every time > > we try to serialize the changes of the transaction. Now, only we > > increase first time when it is not yet serialized. > > > > > > > > 3. > > > ReorderBufferSerializeTXN() > > > { > > > .. > > > /* update the statistics */ > > > rb->spillCount += 1; > > > rb->spillTxns += txn->serialized ? 0 : 1; > > > rb->spillBytes += size; > > > > > > Assert(spilled == txn->nentries_mem); > > > Assert(dlist_is_empty(&txn->changes)); > > > txn->nentries_mem = 0; > > > txn->serialized = true; > > > .. > > > } > > > > > > I am not able to understand the above code. We are setting the > > > serialized parameter a few lines after we check it and increment the > > > spillTxns count. Can you please explain it? > > > > Basically, when the first time we attempt to serialize a transaction, > > txn->serialized will be false, that time we will increment the > > rb->spillTxns and after that set txn->serialized to true. From next > > time onwards if we try to serialize the same transaction we will not > > increment the rb->spillTxns so that we count each transaction only > > once. > > > > Your explanation for both the above comments makes sense to me. Can > you please add some comments along these lines because it is not > apparent why one wants to increase the spillTxns counter when > txn->serialized is false? Ok, I will add comments in the next patch. > > > > > > > Also, isn't spillTxns count bit confusing, because in some cases it > > > will include subtransactions and other cases (where the largest picked > > > transaction is a subtransaction) it won't include it? > > > > I did not understand your comment completely. Basically, every > > transaction which we are serializing we will increase the count first > > time right? whether it is the main transaction or the sub-transaction. > > > > It was not clear to me earlier whether we always increase the > spillTxns counter for subtransactions or not. But now, looking at > code carefully, it is clear that is it is getting increased in every > case. In short, you don't need to do anything for this comment. ok
-- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com