On 22 October 2016 at 19:51, Julien Rouhaud <julien.rouh...@dalibo.com> wrote:
> I just noticed that if track_commit_timestamp is enabled, the
> oldestCommitTsXid and newestCommitTsXid don't persist after a server
> restart, so you can't ask for the commit ts of a transaction that
> committed before the last server start, although the information is
> still available (unless of course if a freeze occured). AFAICT it
> always behave this way.
I initially could'n't see this when tested on 8f1fb7d with a
src/test/recovery/t test script. But it turns out it's OK on immediate
shutdown and crash recovery, but not on clean shutdown and restart.
The attached patch adds a TAP test to src/test/recovery to show this.
If you run the TAP test before recompiling with the fix it'll fail.
"make" to apply the fix, then re-run and it'll succeed. Or just
temporarily roll back the fix with:
git checkout HEAD^1 -- src/backend/access/transam/commit_ts.c
git reset src/backend/access/transam/commit_ts.c
and rebuild to see it fail.
To save time running the recovery suite, just
rm src/test/recovery/00[0-8]*.pl
(It'd be nice to have a prove_check target to run just one test file).
This would explain a few issues I've seen reported with BDR from the
community which I've so far been unable to reproduce, so thanks very
much for the report.
Álvaro, would you mind checking this and committing to HEAD and 9.6?
The commits.c change only should also be committed to 9.5, but not the
TAP test.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From 485ecc6c579073cf71ac88746c53668733bc5cac Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Mon, 24 Oct 2016 12:16:41 +0800
Subject: [PATCH] Preserve access to commit timestamps across restart
An oversight in setting the boundaries of known commit timestamps during
startup caused old commit timestamps to become inaccessible after a server
restart.
Report and bugfix by Julien Rouhaud
Review and commit timestamp tests by Craig Ringer
---
src/backend/access/transam/commit_ts.c | 2 +
src/test/recovery/t/009_committs.pl | 103 +++++++++++++++++++++++++++++++++
2 files changed, 105 insertions(+)
create mode 100644 src/test/recovery/t/009_committs.pl
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index a8d275f..7746578 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -844,6 +844,8 @@ SetCommitTsLimit(TransactionId oldestXact, TransactionId newestXact)
else
{
Assert(ShmemVariableCache->newestCommitTsXid == InvalidTransactionId);
+ ShmemVariableCache->oldestCommitTsXid = oldestXact;
+ ShmemVariableCache->newestCommitTsXid = newestXact;
}
LWLockRelease(CommitTsLock);
}
diff --git a/src/test/recovery/t/009_committs.pl b/src/test/recovery/t/009_committs.pl
new file mode 100644
index 0000000..48f8899
--- /dev/null
+++ b/src/test/recovery/t/009_committs.pl
@@ -0,0 +1,103 @@
+# Testing of logical decoding using SQL interface and/or pg_recvlogical
+use strict;
+use warnings;
+use PostgresNode;
+use TestLib;
+use Test::More tests => 16;
+
+my $node_master = get_new_node('master');
+$node_master->init(allows_streaming => 1);
+$node_master->append_conf(
+ 'postgresql.conf', qq(
+track_commit_timestamp = on
+));
+$node_master->start;
+
+my ($ret, $stdout, $stderr);
+
+($ret, $stdout, $stderr) = $node_master->psql('postgres', qq[SELECT pg_xact_commit_timestamp('0');]);
+is($ret, 3, 'getting ts of InvalidTransactionId reports error');
+like($stderr, qr/cannot retrieve commit timestamp for transaction/, 'expected error from InvalidTransactionId');
+
+($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');
+
+($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');
+
+# 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
+# cts are enabled.
+is($node_master->safe_psql('postgres', qq[SELECT pg_xact_commit_timestamp('3');]), '',
+ 'committs for FirstNormalTransactionId is null');
+
+$node_master->safe_psql('postgres', qq[CREATE TABLE committs_test(x integer, y timestamp with time zone);]);
+
+my $xid = $node_master->safe_psql('postgres', qq[
+ BEGIN;
+ INSERT INTO committs_test(x, y) VALUES (1, current_timestamp);
+ SELECT txid_current();
+ COMMIT;
+]);
+
+my $before_restart_ts = $node_master->safe_psql('postgres', qq[SELECT pg_xact_commit_timestamp('$xid');]);
+ok($before_restart_ts != '' && $before_restart_ts != 'null', 'commit timestamp recorded');
+
+$node_master->stop('immediate');
+$node_master->start;
+
+my $after_crash_ts = $node_master->safe_psql('postgres', qq[SELECT pg_xact_commit_timestamp('$xid');]);
+is($after_crash_ts, $before_restart_ts, 'timestamps before and after crash are equal');
+
+$node_master->stop('fast');
+$node_master->start;
+
+my $after_restart_ts = $node_master->safe_psql('postgres', qq[SELECT pg_xact_commit_timestamp('$xid');]);
+is($after_restart_ts, $before_restart_ts, 'timestamps before and after restart are equal');
+
+# Now disable commit timestamps
+
+$node_master->append_conf(
+ 'postgresql.conf', qq(
+track_commit_timestamp = off
+));
+
+$node_master->stop('fast');
+$node_master->start;
+
+($ret, $stdout, $stderr) = $node_master->psql('postgres', qq[SELECT pg_xact_commit_timestamp('$xid');]);
+is($ret, 3, 'no commit timestamp from enable tx when cts disabled');
+like($stderr, qr/could not get commit timestamp data/, 'expected error from enabled tx when committs disabled');
+
+# Do a tx while cts disabled
+my $xid_disabled = $node_master->safe_psql('postgres', qq[
+ BEGIN;
+ INSERT INTO committs_test(x, y) VALUES (2, current_timestamp);
+ SELECT txid_current();
+ COMMIT;
+]);
+
+# Should be inaccessible
+($ret, $stdout, $stderr) = $node_master->psql('postgres', qq[SELECT pg_xact_commit_timestamp('$xid_disabled');]);
+is($ret, 3, 'no commit timestamp when disabled');
+like($stderr, qr/could not get commit timestamp data/, 'expected error from disabled tx when committs disabled');
+
+# Re-enable, restart and ensure we can still get the old timestamps
+$node_master->append_conf(
+ 'postgresql.conf', qq(
+track_commit_timestamp = on
+));
+
+$node_master->stop('fast');
+$node_master->start;
+
+
+my $after_enable_ts = $node_master->safe_psql('postgres', qq[SELECT pg_xact_commit_timestamp('$xid');]);
+is($after_enable_ts, '', 'timestamp of enabled tx null after re-enable');
+
+my $after_enable_disabled_ts = $node_master->safe_psql('postgres', qq[SELECT pg_xact_commit_timestamp('$xid_disabled');]);
+is($after_enable_disabled_ts, '', 'timestamp of disabled tx null after re-enable');
+
+$node_master->stop;
--
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
