Confused about TransactionIdSetTreeStatus
Hi, hackers
I'm a bit confused about TransactionIdSetTreeStatus, the comment says if
subtransactions cross multiple CLOG pages, it will mark the subxids, that
are on the same page as the main transaction, as sub-committed, and then
set main transaction and subtransactions to committed (step 2).
* Example:
* TransactionId t commits and has subxids t1, t2, t3, t4
* t is on page p1, t1 is also on p1, t2 and t3 are on p2, t4 is on p3
* 1. update pages2-3:
* page2: set t2,t3 as sub-committed
* page3: set t4 as sub-committed
* 2. update page1:
* set t1 as sub-committed,
* then set t as committed,
then set t1 as committed
* 3. update pages2-3:
* page2: set t2,t3 as committed
* page3: set t4 as committed
However, the code marks the main transaction and subtransactions directly
to the committed.
/*
* If this is a commit then we care about doing this correctly (i.e.
* using the subcommitted intermediate status). By here, we know
* we're updating more than one page of clog, so we must mark entries
* that are *not* on the first page so that they show as subcommitted
* before we then return to update the status to fully committed.
*
* To avoid touching the first page twice, skip marking subcommitted
* for the subxids on that first page.
*/
if (status == TRANSACTION_STATUS_COMMITTED)
set_status_by_pages(nsubxids - nsubxids_on_first_page,
subxids + nsubxids_on_first_page,
TRANSACTION_STATUS_SUB_COMMITTED, lsn);
/*
* Now set the parent and subtransactions on same page as the parent,
* if any
*/
pageno = TransactionIdToPage(xid);
TransactionIdSetPageStatus(xid, nsubxids_on_first_page, subxids, status,
lsn, pageno, false);
/*
* Now work through the rest of the subxids one clog page at a time,
* starting from the second page onwards, like we did above.
*/
set_status_by_pages(nsubxids - nsubxids_on_first_page,
subxids + nsubxids_on_first_page,
status, lsn);
Is the comment correct? If not, should we remove it?
--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.
On 25/10/2022 12:02, Japin Li wrote:
I'm a bit confused about TransactionIdSetTreeStatus, the comment says if
subtransactions cross multiple CLOG pages, it will mark the subxids, that
are on the same page as the main transaction, as sub-committed, and then
set main transaction and subtransactions to committed (step 2).* Example:
* TransactionId t commits and has subxids t1, t2, t3, t4
* t is on page p1, t1 is also on p1, t2 and t3 are on p2, t4 is on p3
* 1. update pages2-3:
* page2: set t2,t3 as sub-committed
* page3: set t4 as sub-committed
* 2. update page1:
* set t1 as sub-committed,
* then set t as committed,
then set t1 as committed
* 3. update pages2-3:
* page2: set t2,t3 as committed
* page3: set t4 as committedHowever, the code marks the main transaction and subtransactions directly
to the committed.
Hmm, yeah, step 2 in this example doesn't match reality. We actually set
t and t1 directly as committed. The explanation above that comment is
correct, but the example is not. It used to work the way the example
says, but that was changed in commit
06da3c570f21394003fc392d80f54862f7dec19f. Ironically, that commit also
added the outdated comment.
The correct example would be:
TransactionId t commits and has subxids t1, t2, t3, t4 t is on page p1,
t1 is also on p1, t2 and t3 are on p2, t4 is on p3
1. update pages2-3:
page2: set t2,t3 as sub-committed
page3: set t4 as sub-committed
2. update page1:
page1: set t,t1 as committed,
3. update pages2-3:
page2: set t2,t3 as committed
page3: set t4 as committed
- Heikki
On Tue, 25 Oct 2022 at 22:46, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 25/10/2022 12:02, Japin Li wrote:
However, the code marks the main transaction and subtransactions directly
to the committed.Hmm, yeah, step 2 in this example doesn't match reality. We actually
set t and t1 directly as committed. The explanation above that comment
is correct, but the example is not. It used to work the way the
example says, but that was changed in commit
06da3c570f21394003fc392d80f54862f7dec19f. Ironically, that commit also
added the outdated comment.The correct example would be:
TransactionId t commits and has subxids t1, t2, t3, t4 t is on page
p1, t1 is also on p1, t2 and t3 are on p2, t4 is on p3
1. update pages2-3:
page2: set t2,t3 as sub-committed
page3: set t4 as sub-committed
2. update page1:
page1: set t,t1 as committed,
3. update pages2-3:
page2: set t2,t3 as committed
page3: set t4 as committed
Thanks for your explanation. Attach a patch to remove the outdated comment.
--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.
Attachments:
v1-0001-Remove-outdated-comment-for-TransactionIdSetTreeS.patchtext/x-diffDownload
From c079d8f33a0eb65b8ee9fc1f53c6c358e7ea1516 Mon Sep 17 00:00:00 2001
From: Japin Li <japinli@hotmail.com>
Date: Tue, 25 Oct 2022 23:00:22 +0800
Subject: [PATCH v1 1/1] Remove outdated comment for TransactionIdSetTreeStatus
Commit 06da3c570f eliminates the marking of subtransactions as
SUBCOMMITTED in pg_clog during their commit, however, it introduces
an outdated comment.
---
src/backend/access/transam/clog.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index a7dfcfb4da..77d9894dab 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -146,9 +146,7 @@ static void TransactionIdSetPageStatusInternal(TransactionId xid, int nsubxids,
* page2: set t2,t3 as sub-committed
* page3: set t4 as sub-committed
* 2. update page1:
- * set t1 as sub-committed,
- * then set t as committed,
- then set t1 as committed
+ * page1: set t,t1 as committed
* 3. update pages2-3:
* page2: set t2,t3 as committed
* page3: set t4 as committed
--
2.25.1
On 25/10/2022 18:09, Japin Li wrote:
On Tue, 25 Oct 2022 at 22:46, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 25/10/2022 12:02, Japin Li wrote:
However, the code marks the main transaction and subtransactions directly
to the committed.Hmm, yeah, step 2 in this example doesn't match reality. We actually
set t and t1 directly as committed. The explanation above that comment
is correct, but the example is not. It used to work the way the
example says, but that was changed in commit
06da3c570f21394003fc392d80f54862f7dec19f. Ironically, that commit also
added the outdated comment.The correct example would be:
TransactionId t commits and has subxids t1, t2, t3, t4 t is on page
p1, t1 is also on p1, t2 and t3 are on p2, t4 is on p3
1. update pages2-3:
page2: set t2,t3 as sub-committed
page3: set t4 as sub-committed
2. update page1:
page1: set t,t1 as committed,
3. update pages2-3:
page2: set t2,t3 as committed
page3: set t4 as committedThanks for your explanation. Attach a patch to remove the outdated comment.
Applied, thanks!
- Heikki