Preserve commit timestamps across clean restart

An oversight in setting the boundaries of known commit timestamps during
startup caused old commit timestamps to become inaccessible after a
server restart.

Author and reporter: Julien Rouhaud
Review, test code: Craig Ringer

2 files changed, 139 insertions, 0 deletions

diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index e330105217..67c8bf3cd7 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -843,6 +843,8 @@ SetCommitTsLimit(TransactionId oldestXact, TransactionId newestXact)
 	else
 	{
 		Assert(ShmemVariableCache->newestCommitTsXid == InvalidTransactionId);
+		ShmemVariableCache->oldestCommitTsXid = oldestXact;
+		ShmemVariableCache->newestCommitTsXid = newestXact;
 	}
 	LWLockRelease(CommitTsLock);
 }
diff --git a/src/test/modules/commit_ts/t/004_restart.pl b/src/test/modules/commit_ts/t/004_restart.pl
new file mode 100644
index 0000000000..900e9b7970
--- /dev/null
+++ b/src/test/modules/commit_ts/t/004_restart.pl
@@ -0,0 +1,137 @@
+# Testing of commit timestamps preservation across clean restarts
+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;