public inbox for [email protected]
help / color / mirror / Atom feedFrom: Kingsborough, Alex <[email protected]>
To: [email protected] <[email protected]>
Subject: Null commitTS bug
Date: Fri, 14 Jan 2022 22:49:59 +0000
Message-ID: <[email protected]> (raw)
Hi Hackers,
I've been working with commitTS code recently and during my testing I
found a bug when writing commit timestamps for subxids. Normally for
sub-transaction commit timestamps in TransactionTreeSetCommitTsData(),
we iterate through the subxids until we find one that is on the next commits
page. In the code [1] this is the jth subxid. In SetXidCommitTsInPage()
where we set all the commit timestamps up to but not including the jth
timestamp. The jth timestamp then becomes the head timestamp for next
group of timestamps on the next page. However, if the jth timestamp is
the last subxid (put another way, if the LAST subxid is the FIRST
timestamp on a new page), then the code will break on line 188 [2] and
the timestamp for the last subxid will never be written.
This can be reproduced by enabling track_commit_timestamp and running
a simple loop that has a single sub-transaction like:
psql -t -c 'create table t (id int);'
for i in {1..500}
do
psql -t -c 'begin; insert into t select 1; savepoint a; insert into t select 2;commit'
done
Then querying for NULL commitTS in that table will return that there are
unwritten timestamps:
postgres=# select count(*) from t where pg_xact_commit_timestamp(t.xmin) is NULL;
count
1
(1 row)
The fix for this is very simple
/* if we wrote out all subxids, we're done. /
- if (j + 1 >= nsubxids)
+ if (j >= nsubxids)
break;
[1] https://github.com/postgres/postgres/blame/master/src/backend/access/transam/commit_ts.c#L178
[2] https://github.com/postgres/postgres/blame/master/src/backend/access/transam/commit_ts.c#L188
Attachments:
[application/octet-stream] 0001-commitTS-subxids-bug-fix.patch (921B, ../[email protected]/2-0001-commitTS-subxids-bug-fix.patch)
download | inline diff:
From e49a60353441deffd45dafbd83c7cecf90e24689 Mon Sep 17 00:00:00 2001
From: Alex Kingsborough <[email protected]>
Date: Fri, 14 Jan 2022 21:32:25 +0000
Subject: [PATCH] commitTS subxids bug fix
In commitTS if the last subxid was the first timestamp on a page, that
timestamp would not get written, this change makes sure we write all the
subxid timestamps correctly
---
src/backend/access/transam/commit_ts.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 0985fa155c..3d32507345 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -183,7 +183,7 @@ TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
pageno);
/* if we wrote out all subxids, we're done. */
- if (j + 1 >= nsubxids)
+ if (j >= nsubxids)
break;
/*
--
2.16.6
view thread (2+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Null commitTS bug
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox