Hi all
Today I ran into an issue where commit timestamp lookups were failing with
ERROR: cannot retrieve commit timestamp for transaction 2
which is of course FrozenTransactionId.
TransactionIdGetCommitTsData(...) ERRORs on !TransactionIdIsNormal(),
which I think is wrong. Attached is a patch to make it return 0 for
FrozenTransactionId and BootstrapTransactionId, like it does for xids
that are too old.
Note that the prior behaviour was as designed and has tests to enforce
it. I just think it's wrong, and it's also not documented.
IMO this should be back-patched to 9.6 and, without the TAP test part, to 9.5.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From 52081e9aa91102022d6e853d4e2217308bb230a9 Mon Sep 17 00:00:00 2001
From: Craig Ringer <[email protected]>
Date: Wed, 23 Nov 2016 19:50:50 +0800
Subject: [PATCH] Treat frozen and bootstrap xids as old, not invalid, for
committs
TransactionIdGetCommitTsData() ERROR'd on FrozenTransactionId and
BootstrapTransactionId, which is a surprising outcome for callers
given that it usually returns 0 for xids too old for their commit
timestamp to still be known.
Callers hitting this problem would get an error like:
ERROR: cannot retrieve commit timestamp for transaction 2
Fix it so the frozen and bootstrap XIDs return 0 like other too-old XIDs.
---
src/backend/access/transam/commit_ts.c | 11 +++++++++--
src/test/modules/commit_ts/expected/commit_timestamp.out | 12 ++++++++++--
src/test/modules/commit_ts/t/004_restart.pl | 14 ++++----------
3 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 7746578..a5b270c 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -289,11 +289,18 @@ TransactionIdGetCommitTsData(TransactionId xid, TimestampTz *ts,
TransactionId oldestCommitTsXid;
TransactionId newestCommitTsXid;
- /* error if the given Xid doesn't normally commit */
- if (!TransactionIdIsNormal(xid))
+ if (!TransactionIdIsValid(xid))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("cannot retrieve commit timestamp for transaction %u", xid)));
+ else if (!TransactionIdIsNormal(xid))
+ {
+ /* frozen and bootstrap xids are always committed far in the past */
+ *ts = 0;
+ if (nodeid)
+ *nodeid = 0;
+ return false;
+ }
LWLockAcquire(CommitTsLock, LW_SHARED);
diff --git a/src/test/modules/commit_ts/expected/commit_timestamp.out b/src/test/modules/commit_ts/expected/commit_timestamp.out
index 99f3322..5b7783b 100644
--- a/src/test/modules/commit_ts/expected/commit_timestamp.out
+++ b/src/test/modules/commit_ts/expected/commit_timestamp.out
@@ -28,9 +28,17 @@ DROP TABLE committs_test;
SELECT pg_xact_commit_timestamp('0'::xid);
ERROR: cannot retrieve commit timestamp for transaction 0
SELECT pg_xact_commit_timestamp('1'::xid);
-ERROR: cannot retrieve commit timestamp for transaction 1
+ pg_xact_commit_timestamp
+--------------------------
+
+(1 row)
+
SELECT pg_xact_commit_timestamp('2'::xid);
-ERROR: cannot retrieve commit timestamp for transaction 2
+ pg_xact_commit_timestamp
+--------------------------
+
+(1 row)
+
SELECT x.xid::text::bigint > 0, x.timestamp > '-infinity'::timestamptz, x.timestamp <= now() FROM pg_last_committed_xact() x;
?column? | ?column? | ?column?
----------+----------+----------
diff --git a/src/test/modules/commit_ts/t/004_restart.pl b/src/test/modules/commit_ts/t/004_restart.pl
index 900e9b7..32be07c 100644
--- a/src/test/modules/commit_ts/t/004_restart.pl
+++ b/src/test/modules/commit_ts/t/004_restart.pl
@@ -25,19 +25,13 @@ like(
($ret, $stdout, $stderr) =
$node_master->psql('postgres', qq[SELECT pg_xact_commit_timestamp('1');]);
-is($ret, 3, 'getting ts of BootstrapTransactionId reports error');
-like(
- $stderr,
- qr/cannot retrieve commit timestamp for transaction/,
- 'expected error from BootstrapTransactionId');
+is($ret, 0, 'getting ts of BootstrapTransactionId succeeds');
+is($stdout, '', 'timestamp of BootstrapTransactionId is null');
($ret, $stdout, $stderr) =
$node_master->psql('postgres', qq[SELECT pg_xact_commit_timestamp('2');]);
-is($ret, 3, 'getting ts of FrozenTransactionId reports error');
-like(
- $stderr,
- qr/cannot retrieve commit timestamp for transaction/,
- 'expected error from FrozenTransactionId');
+is($ret, 0, 'getting ts of FrozenTransactionId succeeds');
+is($stdout, '', 'timestamp of FrozenTransactionId is null');
# Since FirstNormalTransactionId will've occurred during initdb, long before we
# enabled commit timestamps, it'll be null since we have no cts data for it but
--
2.5.5
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
