Sawada-san, On Mon, Sep 24, 2018 at 08:28:45AM +0900, Michael Paquier wrote: > Wouldn't it be better to incorporate the new test as part of > 004_restart.pl? This way, we avoid initializing a full instance, which > is always a good thing as that's very costly. The top of this file also > mentions that it tests clean restarts, but it bothers also about crash > recovery.
I have been able to work on this bug, and rewrote the proposed test case as attached. The test can only get on v11 and HEAD. What do you think? -- Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 4754e75436..ddc999b687 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6833,11 +6833,13 @@ StartupXLOG(void)
StartupMultiXact();
/*
- * Ditto commit timestamps. In a standby, we do it if setting is enabled
- * in ControlFile; in a master we base the decision on the GUC itself.
+ * Ditto for commit timestamps. Activate the facility if the setting
+ * is enabled in the control file, as there should be no tracking of
+ * commit timestamps done when the setting was disabled. This facility
+ * can be started or stopped when replaying a XLOG_PARAMETER_CHANGE
+ * record.
*/
- if (ArchiveRecoveryRequested ?
- ControlFile->track_commit_timestamp : track_commit_timestamp)
+ if (ControlFile->track_commit_timestamp)
StartupCommitTs();
/*
diff --git a/src/test/modules/commit_ts/t/004_restart.pl b/src/test/modules/commit_ts/t/004_restart.pl
index daf42d3a02..4efe30a559 100644
--- a/src/test/modules/commit_ts/t/004_restart.pl
+++ b/src/test/modules/commit_ts/t/004_restart.pl
@@ -1,4 +1,4 @@
-# Testing of commit timestamps preservation across clean restarts
+# Testing of commit timestamps preservation across restarts
use strict;
use warnings;
use PostgresNode;
@@ -71,12 +71,36 @@ is($after_restart_ts, $before_restart_ts,
'timestamps before and after restart are equal');
# Now disable commit timestamps
-
$node_master->append_conf('postgresql.conf', 'track_commit_timestamp = off');
-
$node_master->stop('fast');
+
+# Start the server, which generates a XLOG_PARAMETER_CHANGE record where
+# the parameter change is registered.
$node_master->start;
+# Now restart again the server so as no record XLOG_PARAMETER_CHANGE are
+# replayed with the follow-up immediate shutdown.
+$node_master->restart;
+
+# Move commit timestamps across page boundaries. Things should still
+# be able to work across restarts with those transactions committed while
+# track_commit_timestamp is disabled.
+$node_master->safe_psql('postgres',
+qq(CREATE PROCEDURE consume_xid(cnt int)
+AS \$\$
+DECLARE
+ i int;
+ BEGIN
+ FOR i in 1..cnt LOOP
+ EXECUTE 'SELECT txid_current()';
+ COMMIT;
+ END LOOP;
+ END;
+\$\$
+LANGUAGE plpgsql;
+));
+$node_master->safe_psql('postgres', 'CALL consume_xid(2000)');
+
($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');
@@ -106,10 +130,12 @@ like(
# Re-enable, restart and ensure we can still get the old timestamps
$node_master->append_conf('postgresql.conf', 'track_commit_timestamp = on');
-$node_master->stop('fast');
+# An immediate shutdown is used here. At next startup recovery will
+# replay transactions which committed when track_commit_timestamp was
+# disabled, and the facility should be able to work properly.
+$node_master->stop('immediate');
$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');
signature.asc
Description: PGP signature
