On Sun, Oct 13, 2019 at 12:25 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > On Thu, Oct 3, 2019 at 2:43 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > 3. > > + * > > + * While updating the existing change with detoasted tuple data, we need to > > + * update the memory accounting info, because the change size will differ. > > + * Otherwise the accounting may get out of sync, triggering serialization > > + * at unexpected times. > > + * > > + * We simply subtract size of the change before rejiggering the tuple, and > > + * then adding the new size. This makes it look like the change was removed > > + * and then added back, except it only tweaks the accounting info. > > + * > > + * In particular it can't trigger serialization, which would be pointless > > + * anyway as it happens during commit processing right before handing > > + * the change to the output plugin. > > */ > > static void > > ReorderBufferToastReplace(ReorderBuffer *rb, ReorderBufferTXN *txn, > > @@ -3023,6 +3281,13 @@ ReorderBufferToastReplace(ReorderBuffer *rb, > > ReorderBufferTXN *txn, > > if (txn->toast_hash == NULL) > > return; > > > > + /* > > + * We're going modify the size of the change, so to make sure the > > + * accounting is correct we'll make it look like we're removing the > > + * change now (with the old size), and then re-add it at the end. > > + */ > > + ReorderBufferChangeMemoryUpdate(rb, change, false); > > > > It is not very clear why this change is required. Basically, this is done > > at commit time after which actually we shouldn't attempt to spill these > > changes. This is mentioned in comments as well, but it is not clear if > > that is the case, then how and when accounting can create a problem. If > > possible, can you explain it with an example? > > > IIUC, we are keeping the track of the memory in ReorderBuffer which is > common across the transactions. So even if this transaction is > committing and will not spill to dis but we need to keep the memory > accounting correct for the future changes in other transactions. >
You are right. I somehow missed that we need to keep the size computation in sync even during commit for other in-progress transactions in the ReorderBuffer. You can ignore this point or maybe slightly adjust the comment to make it explicit. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
