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 <cr...@2ndquadrant.com> 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 (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers