Re: Included xid in restoring reorder buffer changes from disk log message
On Tue, 10 Oct 2023 at 06:59, Kyotaro Horiguchi wrote:
>
> At Fri, 6 Oct 2023 14:58:13 +0530, vignesh C wrote in
> > On Fri, 30 Apr 2021 at 11:53, Dilip Kumar 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
From bbd3ea2376b76a14d5e59e22e3f36c4637193cab Mon Sep 17 00:00:00 2001
From: Vignesh C
Date: Fri, 6 Oct 2023 14:21:32 +0530
Subject: [PATCH v3] Include xid in restoring reorder buffer changes from disk
log message.
Include xid in restoring reorder buffer changes from disk log message.
---
src/backend/replication/logical/reorderbuffer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 12edc5772a..6ebda5cc5f 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -1442,9 +1442,10 @@ ReorderBufferIterTXNNext(ReorderBuffer *rb, ReorderBufferIterTXNState *state)
dlist_head_element(ReorderBufferChange, node,
&entry->txn->changes);
- elog(DEBUG2, "restored %u/%u changes from disk",
+ elog(DEBUG2, "restored %u/%u changes in XID %u from disk",
(uint32) entry->txn->nentries_mem,
- (uint32) entry->txn->nentries);
+ (uint32) entry->txn->nentries,
+ entry->txn->xid);
Assert(entry->txn->nentries_mem);
/* txn stays the same */
--
2.34.1
Re: Included xid in restoring reorder buffer changes from disk log message
At Fri, 6 Oct 2023 14:58:13 +0530, vignesh C wrote in > On Fri, 30 Apr 2021 at 11:53, Dilip Kumar 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
Re: Included xid in restoring reorder buffer changes from disk log message
On Fri, 30 Apr 2021 at 11:53, Dilip Kumar wrote:
>
> On Thu, Apr 29, 2021 at 9:45 PM vignesh C wrote:
> >
> > 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?
> >
>
> 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
From 5a27c87eb82676c3889d27aa3bddb93744d40ec5 Mon Sep 17 00:00:00 2001
From: Vignesh C
Date: Fri, 6 Oct 2023 14:21:32 +0530
Subject: [PATCH v2] Include xid in restoring reorder buffer changes from disk
log message.
Include xid in restoring reorder buffer changes from disk log message.
---
src/backend/replication/logical/reorderbuffer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 12edc5772a..907bb0039f 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -1442,9 +1442,10 @@ ReorderBufferIterTXNNext(ReorderBuffer *rb, ReorderBufferIterTXNState *state)
dlist_head_element(ReorderBufferChange, node,
&entry->txn->changes);
- elog(DEBUG2, "restored %u/%u changes from disk",
+ elog(DEBUG2, "restored %u/%u changes of XID %u from disk",
(uint32) entry->txn->nentries_mem,
- (uint32) entry->txn->nentries);
+ (uint32) entry->txn->nentries,
+ entry->txn->xid);
Assert(entry->txn->nentries_mem);
/* txn stays the same */
--
2.34.1
Re: Included xid in restoring reorder buffer changes from disk log message
On Thu, Apr 29, 2021 at 9:45 PM vignesh C wrote: > > 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? > 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
