Thread: Included xid in restoring reorder buffer changes from disk log message
Hi,
While debugging one of the logical decoding issues, I found that xid was not included in restoring reorder buffer changes from disk log messages. Attached a patch for it. I felt including the XID will be helpful in debugging. Thoughts?
Regards,
Vignesh
Attachment
On Thu, Apr 29, 2021 at 9:45 PM vignesh C <vignesh21@gmail.com> wrote: > > Hi, > > While debugging one of the logical decoding issues, I found that xid was not included in restoring reorder buffer changesfrom disk log messages. Attached a patch for it. I felt including the XID will be helpful in debugging. Thoughts? > It makes sense to include xid in the debug message, earlier I thought that will it be a good idea to also print the toplevel_xid. But I think it is for debug purposes and only we have the xid we can fetch the other parameters so maybe adding xid is good enough. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
On Fri, 30 Apr 2021 at 11:53, Dilip Kumar <dilipbalaut@gmail.com> wrote: > > On Thu, Apr 29, 2021 at 9:45 PM vignesh C <vignesh21@gmail.com> wrote: > > > > Hi, > > > > While debugging one of the logical decoding issues, I found that xid was not included in restoring reorder buffer changesfrom disk log messages. Attached a patch for it. I felt including the XID will be helpful in debugging. Thoughts? > > > > It makes sense to include xid in the debug message, earlier I thought > that will it be a good idea to also print the toplevel_xid. But I > think it is for debug purposes and only we have the xid we can fetch > the other parameters so maybe adding xid is good enough. While having a look at the reorderbuffer code, I noticed that this changes were still not committed. Here is a rebased version of the patch. Regards, Vignesh
Attachment
Re: Included xid in restoring reorder buffer changes from disk log message
From
Kyotaro Horiguchi
Date:
At Fri, 6 Oct 2023 14:58:13 +0530, vignesh C <vignesh21@gmail.com> wrote in > On Fri, 30 Apr 2021 at 11:53, Dilip Kumar <dilipbalaut@gmail.com> wrote: > > It makes sense to include xid in the debug message, earlier I thought > > that will it be a good idea to also print the toplevel_xid. But I > > think it is for debug purposes and only we have the xid we can fetch > > the other parameters so maybe adding xid is good enough. +1 > While having a look at the reorderbuffer code, I noticed that this > changes were still not committed. > Here is a rebased version of the patch. While this patch makes the following change on the de-serializing side; - elog(DEBUG2, "restored %u/%u changes from disk", + elog(DEBUG2, "restored %u/%u changes of XID %u from disk", the counter part ReorderBufferSerializeTXN() has the following message. > elog(DEBUG2, "spill %u changes in XID %u to disk", > (uint32) txn->nentries_mem, txn->xid); It might be preferable for those two messages to have a corresponding appearance. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
On Tue, 10 Oct 2023 at 06:59, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote: > > At Fri, 6 Oct 2023 14:58:13 +0530, vignesh C <vignesh21@gmail.com> wrote in > > On Fri, 30 Apr 2021 at 11:53, Dilip Kumar <dilipbalaut@gmail.com> wrote: > > > It makes sense to include xid in the debug message, earlier I thought > > > that will it be a good idea to also print the toplevel_xid. But I > > > think it is for debug purposes and only we have the xid we can fetch > > > the other parameters so maybe adding xid is good enough. > > +1 > > > While having a look at the reorderbuffer code, I noticed that this > > changes were still not committed. > > Here is a rebased version of the patch. > > While this patch makes the following change on the de-serializing side; > > - elog(DEBUG2, "restored %u/%u changes from disk", > + elog(DEBUG2, "restored %u/%u changes of XID %u from disk", > > the counter part ReorderBufferSerializeTXN() has the following > message. > > > elog(DEBUG2, "spill %u changes in XID %u to disk", > > (uint32) txn->nentries_mem, txn->xid); > > It might be preferable for those two messages to have a corresponding > appearance. We cannot include nentries in ReorderBufferSerializeTXN as the number of entries will not be known at that time. Modified to keep it consistent by changing it to "... changes in XID ...". Attached v3 patch has the changes for the same. Regards, Vignesh