Hi,
Followed all recommendations.
----------------
Кому: 'ls7777' ([email protected]);
Тема: Patch for migration of the pg_commit_ts directory;
09.10.2025, 11:22, "Hayato Kuroda (Fujitsu)" <[email protected]>:
Hi,
Thanks for updating the patch and sorry for the late reply. Here are my comments.
01.
```
+ prep_status("Checking for pg_commit_ts");
```
I think we must clarify which node is being checked. Something like:
Checking for new cluster configuration for commit timestamp
02.
```
}
-
/* we have the result of cmd in "output". so parse it line by line now */
```
This change is not needed.
03.
```
+ /*
+ * Copy pg_commit_ts
+ */
```
I feel it can be removed or have more meanings. Something lile:
Copy old commit_timestamp data to new, if available.
04.
Regarding the test,
05.
```
+sub command_output
```
Can run_command() be used instead of defining new function?
06.
```
+$old->command_ok([ 'pgbench', '-i', '-s', '1' ], 'init pgbench');
+$old->command_ok([ 'pgbench', '-t', '1', '-j', '1' ], 'pgbench it');
```
I think no need to use pgbench anymore. Can we define tables and insert tuples
by myself?
07.
```
+command_fails(
+ [
+ 'pg_upgrade', '--no-sync',
+ '-d', $old->data_dir,
+ '-D', $new->data_dir,
+ '-b', $old->config_data('--bindir'),
+ '-B', $new->config_data('--bindir'),
+ '-s', $new->host,
+ '-p', $old->port,
+ '-P', $new->port,
+ $mode
+ ],
+ 'run of pg_upgrade fails with mismatch parameter track_commit_timestamp');
```
According to other test files, we do not use the shorten option.
Also, please verify the actual output by command_ok_or_fails_like() or command_checks_all().
08.
```
+sub xact_commit_ts
```
Not sure, but this function is introduced because we have 100 transactions. Can we omit this now?
09.
```
+# The new cluster should contain timestamps created during the pg_upgrade and
+# some more created by the pgbench.
+#
+print "\nCommit timestamp only in new cluster:\n";
+for my $line (@b) {
+ print "$line\n" unless $h1{$line};
+}
```
I don't think this is needed because the output is not verified.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
From 254cae551bbde68c4932d61dc96167fee9f97155 Mon Sep 17 00:00:00 2001 From: Sergey Levin <[email protected]> Date: Sat, 4 Oct 2025 20:22:46 +0500 Subject: [PATCH] Migration of the pg_commit_ts directory
---
src/bin/pg_upgrade/check.c | 22 ++++++
src/bin/pg_upgrade/controldata.c | 20 +++++
src/bin/pg_upgrade/meson.build | 1 +
src/bin/pg_upgrade/pg_upgrade.c | 20 ++++-
src/bin/pg_upgrade/pg_upgrade.h | 2 +
.../pg_upgrade/t/007_transfer_commit_ts.pl | 75 +++++++++++++++++++
6 files changed, 137 insertions(+), 3 deletions(-)
create mode 100644 src/bin/pg_upgrade/t/007_transfer_commit_ts.pl
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 1e17d64b3ec..c5e29485416 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -32,6 +32,7 @@ static void check_new_cluster_replication_slots(void);
static void check_new_cluster_subscription_configuration(void);
static void check_old_cluster_for_valid_slots(void);
static void check_old_cluster_subscription_state(void);
+static void check_new_cluster_pg_commit_ts(void);
/*
* DataTypesUsageChecks - definitions of data type checks for the old cluster
@@ -767,9 +768,30 @@ check_new_cluster(void)
check_new_cluster_replication_slots();
check_new_cluster_subscription_configuration();
+
+ check_new_cluster_pg_commit_ts();
+
}
+void
+check_new_cluster_pg_commit_ts(void)
+{
+ PGconn *conn;
+ PGresult *res;
+ bool track_commit_timestamp_on;
+
+ prep_status("Checking for new cluster configuration for commit timestamp");
+ conn = connectToServer(&new_cluster, "template1");
+ res = executeQueryOrDie(conn, "SELECT setting "
+ "FROM pg_settings WHERE name = 'track_commit_timestamp'");
+ track_commit_timestamp_on = strcmp(PQgetvalue(res, 0, 0), "on") == 0;
+ PQclear(res);
+ PQfinish(conn);
+ if (old_cluster.controldata.chkpnt_newstCommitTsxid > 0 && !track_commit_timestamp_on)
+ pg_fatal("\"track_commit_timestamp\" must be \"on\" but is set to \"off\"");
+ check_ok();
+}
void
report_clusters_compatible(void)
{
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 90cef0864de..594dc1c0a9f 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -321,6 +321,26 @@ get_control_data(ClusterInfo *cluster)
cluster->controldata.chkpnt_nxtmulti = str2uint(p);
got_multi = true;
}
+ else if ((p = strstr(bufin, "Latest checkpoint's oldestCommitTsXid:")) != NULL)
+ {
+ p = strchr(p, ':');
+
+ if (p == NULL || strlen(p) <= 1)
+ pg_fatal("%d: controldata retrieval problem", __LINE__);
+
+ p++; /* remove ':' char */
+ cluster->controldata.chkpnt_oldstCommitTsxid = str2uint(p);
+ }
+ else if ((p = strstr(bufin, "Latest checkpoint's newestCommitTsXid:")) != NULL)
+ {
+ p = strchr(p, ':');
+
+ if (p == NULL || strlen(p) <= 1)
+ pg_fatal("%d: controldata retrieval problem", __LINE__);
+
+ p++; /* remove ':' char */
+ cluster->controldata.chkpnt_newstCommitTsxid = str2uint(p);
+ }
else if ((p = strstr(bufin, "Latest checkpoint's oldestXID:")) != NULL)
{
p = strchr(p, ':');
diff --git a/src/bin/pg_upgrade/meson.build b/src/bin/pg_upgrade/meson.build
index ac992f0d14b..030618152d0 100644
--- a/src/bin/pg_upgrade/meson.build
+++ b/src/bin/pg_upgrade/meson.build
@@ -47,6 +47,7 @@ tests += {
't/004_subscription.pl',
't/005_char_signedness.pl',
't/006_transfer_modes.pl',
+ 't/007_transfer_commit_ts.pl',
],
'test_kwargs': {'priority': 40}, # pg_upgrade tests are slow
},
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 490e98fa26f..c46a4432fd2 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -772,6 +772,8 @@ copy_subdir_files(const char *old_subdir, const char *new_subdir)
static void
copy_xact_xlog_xid(void)
{
+ bool is_copy_commit_ts;
+
/*
* Copy old commit logs to new data dir. pg_clog has been renamed to
* pg_xact in post-10 clusters.
@@ -781,6 +783,15 @@ copy_xact_xlog_xid(void)
GET_MAJOR_VERSION(new_cluster.major_version) <= 906 ?
"pg_clog" : "pg_xact");
+ /*
+ * Copy old commit_timestamp data to new, if available.
+ */
+ is_copy_commit_ts = old_cluster.controldata.chkpnt_oldstCommitTsxid > 0
+ && old_cluster.controldata.chkpnt_newstCommitTsxid > 0;
+
+ if (is_copy_commit_ts)
+ copy_subdir_files("pg_commit_ts", "pg_commit_ts");
+
prep_status("Setting oldest XID for new cluster");
exec_prog(UTILITY_LOG_FILE, NULL, true, true,
"\"%s/pg_resetwal\" -f -u %u \"%s\"",
@@ -798,12 +809,15 @@ copy_xact_xlog_xid(void)
"\"%s/pg_resetwal\" -f -e %u \"%s\"",
new_cluster.bindir, old_cluster.controldata.chkpnt_nxtepoch,
new_cluster.pgdata);
- /* must reset commit timestamp limits also */
+
+ /*
+ * must reset commit timestamp limits also or copy from the old cluster
+ */
exec_prog(UTILITY_LOG_FILE, NULL, true, true,
"\"%s/pg_resetwal\" -f -c %u,%u \"%s\"",
new_cluster.bindir,
- old_cluster.controldata.chkpnt_nxtxid,
- old_cluster.controldata.chkpnt_nxtxid,
+ is_copy_commit_ts ? old_cluster.controldata.chkpnt_oldstCommitTsxid : old_cluster.controldata.chkpnt_nxtxid,
+ is_copy_commit_ts ? old_cluster.controldata.chkpnt_newstCommitTsxid : old_cluster.controldata.chkpnt_nxtxid,
new_cluster.pgdata);
check_ok();
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 0ef47be0dc1..83fdbc44b13 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -238,6 +238,8 @@ typedef struct
uint32 chkpnt_nxtmxoff;
uint32 chkpnt_oldstMulti;
uint32 chkpnt_oldstxid;
+ uint32 chkpnt_oldstCommitTsxid;
+ uint32 chkpnt_newstCommitTsxid;
uint32 align;
uint32 blocksz;
uint32 largesz;
diff --git a/src/bin/pg_upgrade/t/007_transfer_commit_ts.pl b/src/bin/pg_upgrade/t/007_transfer_commit_ts.pl
new file mode 100644
index 00000000000..1b75bf17e8c
--- /dev/null
+++ b/src/bin/pg_upgrade/t/007_transfer_commit_ts.pl
@@ -0,0 +1,75 @@
+# Copyright (c) 2025, PostgreSQL Global Development Group
+
+# Tests for transfer pg_commit_ts directory.
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Can be changed to test the other modes
+my $mode = $ENV{PG_TEST_PG_UPGRADE_MODE} || '--copy';
+
+# Initialize old cluster
+my $old = PostgreSQL::Test::Cluster->new('old');
+$old->init;
+$old->append_conf('postgresql.conf', 'track_commit_timestamp = on');
+$old->start;
+my $resold = $old->safe_psql('postgres', "
+ create table a(a int);
+ select xid,timestamp from pg_last_committed_xact();
+ ");
+
+my ($xid,$ts) = $resold =~ /\s*(\d+)\s*\|(.*)/;
+$old->stop;
+
+# Initialize new cluster
+my $new = PostgreSQL::Test::Cluster->new('new');
+$new->init;
+command_checks_all(
+ [
+ 'pg_upgrade', '--no-sync',
+ '--old-datadir' => $old->data_dir,
+ '--new-datadir' => $new->data_dir,
+ '--old-bindir' => $old->config_data('--bindir'),
+ '--new-bindir' => $new->config_data('--bindir'),
+ '--socketdir' => $new->host,
+ '--old-port' => $old->port,
+ '--new-port' => $new->port,
+ $mode
+ ],
+ 1,
+ [qr{"track_commit_timestamp" must be "on" but is set to "off"}],
+ [],
+ 'run of pg_upgrade for mismatch parameter track_commit_timestamp');
+
+
+$new->append_conf('postgresql.conf', 'track_commit_timestamp = on');
+
+command_checks_all(
+ [
+ 'pg_upgrade', '--no-sync',
+ '--old-datadir' => $old->data_dir,
+ '--new-datadir' => $new->data_dir,
+ '--old-bindir' => $old->config_data('--bindir'),
+ '--new-bindir' => $new->config_data('--bindir'),
+ '--socketdir' => $new->host,
+ '--old-port' => $old->port,
+ '--new-port' => $new->port,
+ $mode
+ ],
+ 0,
+ [],
+ [],
+ 'run of pg_upgrade ok');
+
+$new->start;
+my $resnew = $new->safe_psql('postgres', "
+ select $xid,pg_xact_commit_timestamp(${xid}::text::xid);
+ ");
+$new->stop;
+ok($resold eq $resnew, "timestamp transferred successfully");
+
+done_testing();
--
2.42.2
