public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 6/6] 0003 review
2+ messages / 2 participants
[nested] [flat]
* [PATCH 6/6] 0003 review
@ 2021-02-15 15:36 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Tomas Vondra @ 2021-02-15 15:36 UTC (permalink / raw)
---
src/test/regress/input/copy.source | 4 ++--
src/test/regress/output/copy.source | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/test/regress/input/copy.source b/src/test/regress/input/copy.source
index ddde33e7cc..4ee1b26326 100644
--- a/src/test/regress/input/copy.source
+++ b/src/test/regress/input/copy.source
@@ -204,7 +204,7 @@ drop table parted_copytest;
--
-- progress reporting
---
+--
-- setup
-- reuse employer datatype, that has a small sized data set
@@ -221,7 +221,7 @@ create function notice_after_progress_reporting() returns trigger AS
$$
declare report record;
begin
- -- We cannot expect 'pid' nor 'relid' to be consistent over runs due to
+ -- We cannot expect 'pid' nor 'relid' to be consistent over runs due to
-- variance in system process ids, and concurrency in runs of tests.
-- Additionally, due to the usage of this test in pg_regress, the 'datid'
-- also is not consistent between runs.
diff --git a/src/test/regress/output/copy.source b/src/test/regress/output/copy.source
index 60f4206aa1..8ebfb7ae81 100644
--- a/src/test/regress/output/copy.source
+++ b/src/test/regress/output/copy.source
@@ -167,7 +167,7 @@ select * from parted_copytest where b = 2;
drop table parted_copytest;
--
-- progress reporting
---
+--
-- setup
-- reuse employer datatype, that has a small sized data set
create table progress_reporting (
@@ -181,7 +181,7 @@ create function notice_after_progress_reporting() returns trigger AS
$$
declare report record;
begin
- -- We cannot expect 'pid' nor 'relid' to be consistent over runs due to
+ -- We cannot expect 'pid' nor 'relid' to be consistent over runs due to
-- variance in system process ids, and concurrency in runs of tests.
-- Additionally, due to the usage of this test in pg_regress, the 'datid'
-- also is not consistent between runs.
--
2.26.2
--------------DA8D60BF026F7ACE69A9AEE5--
^ permalink raw reply [nested|flat] 2+ messages in thread
* Null commitTS bug
@ 2022-01-14 22:49 Kingsborough, Alex <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Kingsborough, Alex @ 2022-01-14 22:49 UTC (permalink / raw)
To: [email protected] <[email protected]>
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
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2022-01-14 22:49 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-02-15 15:36 [PATCH 6/6] 0003 review Tomas Vondra <[email protected]>
2022-01-14 22:49 Null commitTS bug Kingsborough, Alex <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox