On Fri, Dec 27, 2019 at 8:37 PM Alvaro Herrera <alvhe...@2ndquadrant.com> wrote: > > On 2019-Dec-27, vignesh C wrote: > > > I felt amit solution also solves the problem. Attached patch has the > > fix based on the solution proposed. > > Thoughts? > > This seems a sensible fix to me, though I didn't try to reproduce the > failure. > > > @@ -2472,6 +2457,7 @@ ReorderBufferSerializeTXN(ReorderBuffer *rb, > > ReorderBufferTXN *txn) > > } > > > > ReorderBufferSerializeChange(rb, txn, fd, change); > > + txn->final_lsn = change->lsn; > > dlist_delete(&change->node); > > ReorderBufferReturnChange(rb, change); > > Should this be done insider ReorderBufferSerializeChange itself, instead > of in its caller? >
makes sense. But, I think we should add a comment specifying the reason why it is important to set final_lsn while serializing the change. > Also, would it be sane to verify that the TXN > doesn't already have a newer final_lsn? Maybe as an Assert. > I don't think this is a good idea because we update the final_lsn with commit_lsn in ReorderBufferCommit after which we can try to serialize the remaining changes. Instead, we should update it only if the change_lsn value is greater than final_lsn. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
