public inbox for [email protected]
help / color / mirror / Atom feedFrom: Hayato Kuroda (Fujitsu) <[email protected]>
To: 'Amit Kapila' <[email protected]>
Cc: ls7777 <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Subject: RE: Patch for migration of the pg_commit_ts directory
Date: Mon, 23 Feb 2026 04:41:30 +0000
Message-ID: <OS9PR01MB12149F6E69BDF8ED6EF6F467AF577A@OS9PR01MB12149.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAA4eK1JFtm45nqE_4f=tJPsvLoAssSBA-RUqkPGhSZbQ7vSZ=g@mail.gmail.com>
References: <[email protected]>
<OSCPR01MB1496660969357E7D16EF4FF12F5E6A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<[email protected]>
<OSCPR01MB14966B4CAB3CC8AE5F8DAD57FF5E7A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<CAA4eK1+Zayox=x84upet_gc4hrCKSwZSpnO9=Q4vH8rrJbsBOg@mail.gmail.com>
<CACG=ezaFdL4EM8bdrHTRVHt5BmxxWGXcPiDr6Qm00Qj+S_Y+vQ@mail.gmail.com>
<[email protected]>
<OSCPR01MB14966A30D6251B0AE867FEE54F5E0A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<[email protected]>
<OSCPR01MB14966709F525419547BA3F99DF5E0A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<[email protected]>
<OSCPR01MB14966359349E2A205DFB2D6CAF5EEA@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<[email protected]>
<OSCPR01MB14966282B927E003022710B76F5EBA@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<[email protected]>
<OSCPR01MB14966D4347F7C83697B074223F5E8A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<TY7PR01MB14554CC93E03A125BEE334C1EF586A@TY7PR01MB14554.jpnprd01.prod.outlook.com>
<CAA4eK1KMuhaH=QgzJCwQgawuybosq-JMWWT9qSqyMZNV1MWjtg@mail.gmail.com>
<OS9PR01MB121493384C07100ED5AF5E1E6F568A@OS9PR01MB12149.jpnprd01.prod.outlook.com>
<CAA4eK1JFtm45nqE_4f=tJPsvLoAssSBA-RUqkPGhSZbQ7vSZ=g@mail.gmail.com>
Dear Amit,
> > Right, and code comments [a] should be also updated.
> >
>
> So, leaving aside update_delete, copying commit_ts could be helpful to
> detect some other conflicts. You may want to once test the same and
> show it here as part of use case establishment.
I confirmed that {update|delete}_origin_differs could be detected with the
following steps.
0.
Constructed pub-sub replication system. track_commit_timestamp=on was set on the
subscriber, and the table below was defined on both clusters
```
Table "public.employee"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
id | integer | | not null |
salary | integer | | |
Indexes:
"employee_pkey" PRIMARY KEY, btree (id)
```
1.
Inserted a tuple on the publisher
```
pub=# INSERT INTO employee VALUES (1, 100);
INSERT 0 1
```
2.
UPDATEd the replicated tuple on the subscriber. Confirmed that commit timestamps
were stored.
```
sub=# SELECT * FROM pg_last_committed_xact();
xid | timestamp | roident
-----+-------------------------------+---------
738 | 2026-02-23 13:17:19.263146+09 | 1
(1 row)
sub=# UPDATE employee SET salary = 10 WHERE id = 1;
UPDATE 1
sub=# SELECT * FROM pg_last_committed_xact();
xid | timestamp | roident
-----+-------------------------------+---------
739 | 2026-02-23 13:17:33.230773+09 | 0
(1 row)
```
3.
Ran pg_upgrade to upgrade the subscriber. The new cluster must also set
track_commit_timestamp to on.
4.
Confirmed commit timestamps could be migrated.
```
new=# SELECT * FROM pg_xact_commit_timestamp_origin('739');
timestamp | roident
-------------------------------+---------
2026-02-23 13:17:33.230773+09 | 0
(1 row)
```
5.
UPDATEd the tuple on the publisher.
```
pub=# UPDATE employee SET salary = 200 WHERE id = 1;
UPDATE 1
```
6.
update_origin_differs conflict was detected on the new subscriber.
```
LOG: conflict detected on relation "public.employee": conflict=update_origin_differs
DETAIL: Updating the row that was modified locally in transaction 739 at 2026-02-23 13:17:33.230773+09: local row (1, 10), remote row (1, 200), replica identity (id)=(1).
CONTEXT: processing remote data for replication origin "pg_16402" during message type "UPDATE" for replication target relation "public.employee" in transaction 745, finished at 0/018A4C6
```
One debatable point is whether we should include this in the TAP test. Personally
It's not necessary to simplify the test; either one is OK.
Not sure it is OK, but I created updated patches and considered a commit message.
0001 was not changed from the original, and 0002 addressed comments from you and
contained the commit message. For now I added my name as one of the author, but
OK to be listed as reviewer. Most of parts were written by Sergey.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] v9-0001-Migration-of-the-pg_commit_ts-directory.patch (8.1K, ../OS9PR01MB12149F6E69BDF8ED6EF6F467AF577A@OS9PR01MB12149.jpnprd01.prod.outlook.com/2-v9-0001-Migration-of-the-pg_commit_ts-directory.patch)
download | inline diff:
From 596135c7e78d47d577c115cb56d62b98d82999f7 Mon Sep 17 00:00:00 2001
From: Sergey Levin <[email protected]>
Date: Sat, 4 Oct 2025 20:22:46 +0500
Subject: [PATCH v9 1/2] Migration of the pg_commit_ts directory
---
src/bin/pg_upgrade/check.c | 24 +++++++
src/bin/pg_upgrade/controldata.c | 20 ++++++
src/bin/pg_upgrade/meson.build | 1 +
src/bin/pg_upgrade/pg_upgrade.c | 29 +++++++-
src/bin/pg_upgrade/pg_upgrade.h | 2 +
.../pg_upgrade/t/008_transfer_commit_ts.pl | 67 +++++++++++++++++++
6 files changed, 140 insertions(+), 3 deletions(-)
create mode 100644 src/bin/pg_upgrade/t/008_transfer_commit_ts.pl
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 5c73773bf0e..f10753b342e 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -34,6 +34,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
@@ -782,9 +783,32 @@ 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 (!track_commit_timestamp_on &&
+ old_cluster.controldata.chkpnt_newstCommitTsxid > 0)
+ 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 aa6e8b4de5d..fa8b28adf43 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 49b1b624f25..b27477c3f8a 100644
--- a/src/bin/pg_upgrade/meson.build
+++ b/src/bin/pg_upgrade/meson.build
@@ -51,6 +51,7 @@ tests += {
't/005_char_signedness.pl',
't/006_transfer_modes.pl',
't/007_multixact_conversion.pl',
+ 't/008_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 2127d297bfe..b761f6c86c4 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -773,6 +773,10 @@ copy_subdir_files(const char *old_subdir, const char *new_subdir)
static void
copy_xact_xlog_xid(void)
{
+ bool is_copy_commit_ts;
+ uint32 oldest_xid,
+ newest_xid;
+
/*
* Copy old commit logs to new data dir. pg_clog has been renamed to
* pg_xact in post-10 clusters.
@@ -782,6 +786,22 @@ 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");
+ oldest_xid = old_cluster.controldata.chkpnt_oldstCommitTsxid;
+ newest_xid = old_cluster.controldata.chkpnt_newstCommitTsxid;
+ }
+ else
+ oldest_xid = newest_xid = old_cluster.controldata.chkpnt_nxtxid;
+
prep_status("Setting oldest XID for new cluster");
exec_prog(UTILITY_LOG_FILE, NULL, true, true,
"\"%s/pg_resetwal\" -f -u %u \"%s\"",
@@ -799,12 +819,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,
+ oldest_xid,
+ newest_xid,
new_cluster.pgdata);
check_ok();
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index ec018e4f292..938413521cd 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -245,6 +245,8 @@ typedef struct
uint64 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/008_transfer_commit_ts.pl b/src/bin/pg_upgrade/t/008_transfer_commit_ts.pl
new file mode 100644
index 00000000000..a9f0ff4ba8f
--- /dev/null
+++ b/src/bin/pg_upgrade/t/008_transfer_commit_ts.pl
@@ -0,0 +1,67 @@
+# Copyright (c) 2026, 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', qq{
+ create table a(a int);
+ select xid,timestamp from pg_last_committed_xact();
+});
+
+my ($xid) = $resold =~ /\s*(\d+)\s*\|.*/;
+$old->stop;
+
+# Initialize new cluster
+my $new = PostgreSQL::Test::Cluster->new('new');
+$new->init;
+
+# Setup a common pg_upgrade command to be used by all the test cases
+my @pg_upgrade_cmd = (
+ '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);
+
+# In a VPATH build, we'll be started in the source directory, but we want
+# to run pg_upgrade in the build directory so that any files generated finish
+# in it, like delete_old_cluster.{sh,bat}.
+chdir ${PostgreSQL::Test::Utils::tmp_check};
+
+command_checks_all(
+ [@pg_upgrade_cmd], 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_ok([@pg_upgrade_cmd], 'run of pg_upgrade ok');
+
+$new->start;
+my $resnew = $new->safe_psql(
+ 'postgres', qq{
+ select $xid,pg_xact_commit_timestamp(${xid}::text::xid);
+});
+$new->stop;
+ok($resold eq $resnew, "timestamp transferred successfully");
+
+done_testing();
--
2.43.0
[application/octet-stream] v9-0002-pg_upgrade-transfer-commit-timestamps-to-the-new-.patch (4.6K, ../OS9PR01MB12149F6E69BDF8ED6EF6F467AF577A@OS9PR01MB12149.jpnprd01.prod.outlook.com/3-v9-0002-pg_upgrade-transfer-commit-timestamps-to-the-new-.patch)
download | inline diff:
From dcc2e14b3fc4a738a350dd193878b8365fb26ea7 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Mon, 23 Feb 2026 12:14:40 +0900
Subject: [PATCH v9 2/2] pg_upgrade: transfer commit timestamps to the new
cluster
This commit preserves commit timestamps during an upgrade. The advantage of this
commit is not only to provide committed transaction information after the upgrade,
but also to allow detecting {UPDATE|DELETE}_ORIGIN_DIFFERS conflicts for tuples
that were modified before the upgrade.
Files in the pg_commit_ts directory are copied when track_commit_timestamp=on.
Also, pg_resetwal specifies the oldest and newest transaction IDs to the new
cluster.
If the old cluster enables tracking commit timestamps but the new cluster does
not, the pg_upgrade fails to avoid missing them.
Author: Sergey Levin <[email protected]>
Author: Hayato Kuroda <[email protected]>
Reviewed-by: Maxim Orlov <orlovmg.gmail.com>
---
doc/src/sgml/logical-replication.sgml | 8 ++++----
src/bin/pg_upgrade/check.c | 6 +++---
src/bin/pg_upgrade/pg_upgrade.c | 12 +++++-------
3 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 5028fe9af09..3c0db3ab233 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -2814,11 +2814,11 @@ CONTEXT: processing remote data for replication origin "pg_16395" during "INSER
<note>
<para>
- Commit timestamps and origin data are not preserved during the upgrade.
- As a result, even if
+ Parameters for physical replication slots are not preserved during the
+ upgrade. As a result, even if
<link linkend="sql-createsubscription-params-with-retain-dead-tuples"><literal>retain_dead_tuples</literal></link>
- is enabled, the upgraded subscriber may be unable to detect conflicts or
- log relevant commit timestamps and origins when applying changes from the
+ is enabled, the upgraded subscriber may be unable to detect
+ <literal>update_deleted</literal> conflicts when applying changes from the
publisher occurred before the upgrade. Additionally, immediately after the
upgrade, the vacuum may remove the deleted rows that are required for
conflict detection. This can affect the changes that were not replicated
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index f10753b342e..32352752095 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -793,17 +793,17 @@ check_new_cluster_pg_commit_ts(void)
{
PGconn *conn;
PGresult *res;
- bool track_commit_timestamp_on;
+ bool commit_ts_is_enabled;
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;
+ commit_ts_is_enabled = strcmp(PQgetvalue(res, 0, 0), "on") == 0;
PQclear(res);
PQfinish(conn);
- if (!track_commit_timestamp_on &&
+ if (!commit_ts_is_enabled &&
old_cluster.controldata.chkpnt_newstCommitTsxid > 0)
pg_fatal("\"track_commit_timestamp\" must be \"on\" but is set to \"off\"");
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index b761f6c86c4..bfec731ce6b 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -216,13 +216,11 @@ main(int argc, char **argv)
* as it only retains the dead tuples. It is created here for consistency.
* Note that the new conflict detection slot uses the latest transaction
* ID as xmin, so it cannot protect dead tuples that existed before the
- * upgrade. Additionally, commit timestamps and origin data are not
- * preserved during the upgrade. So, even after creating the slot, the
- * upgraded subscriber may be unable to detect conflicts or log relevant
- * commit timestamps and origins when applying changes from the publisher
- * occurred before the upgrade especially if those changes were not
- * replicated. It can only protect tuples that might be deleted after the
- * new cluster starts.
+ * upgrade. It means even after creating the slot, the upgraded subscriber
+ * may be unable to detect update_deleted conflicts when applying changes
+ * from the publisher occurred before the upgrade especially if those
+ * changes were not replicated. It can only protect tuples that might be
+ * deleted after the new cluster starts.
*/
if (migrate_logical_slots || old_cluster.sub_retain_dead_tuples)
{
--
2.43.0
view thread (13+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: RE: Patch for migration of the pg_commit_ts directory
In-Reply-To: <OS9PR01MB12149F6E69BDF8ED6EF6F467AF577A@OS9PR01MB12149.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox