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
Attachments:
0001-Included-xid-in-restoring-reorder-buffer-changes-fro.patchtext/x-patch; charset=US-ASCII; name=0001-Included-xid-in-restoring-reorder-buffer-changes-fro.patchDownload
From 9d3ee45b7b2c0d625af888579035a0fb9a1e512c Mon Sep 17 00:00:00 2001
From: vignesh <vignesh21@gmail.com>
Date: Thu, 29 Apr 2021 21:38:09 +0530
Subject: [PATCH] Included xid in restoring reorder buffer changes from disk.
Included xid in restoring reorder buffer changes from disk.
---
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 cdf46a36af..b00f7f4801 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -1381,9 +1381,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,
+ (uint32) entry->txn->xid);
Assert(entry->txn->nentries_mem);
/* txn stays the same */
--
2.25.1
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 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
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 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
Attachments:
v2-0001-Include-xid-in-restoring-reorder-buffer-changes-f.patchtext/x-patch; charset=US-ASCII; name=v2-0001-Include-xid-in-restoring-reorder-buffer-changes-f.patchDownload
From 5a27c87eb82676c3889d27aa3bddb93744d40ec5 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
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
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
Attachments:
v3-0001-Include-xid-in-restoring-reorder-buffer-changes-f.patchtext/x-patch; charset=US-ASCII; name=v3-0001-Include-xid-in-restoring-reorder-buffer-changes-f.patchDownload
From bbd3ea2376b76a14d5e59e22e3f36c4637193cab Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
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