Subject: Patch for migration of the pg_commit_ts directory

From: ls7777 <[email-masked]>
Date: Thu, 03 Apr 2025 23:27:32 +0500

Good afternoon!

During pg_upgrade migration, the pg_commit_ts directory is not copied when track_commit_timestamp = on is set. There is a patch in the attachment that fixes this.

Contents & Purpose

==================

The patch copies the pg_commit_ts directory of the old cluster and updates the Latest checkpoints oldestCommitTsXid and Latest checkpoint's newestCommitTsXid for the new cluster. If required.

Initial Run

===========

The patch can be applied on the master branch.

I don't understand what tests are needed for such a patch. The patch is very simple.

Performance

===========

The pg_upgrade operation time increases by the time the pg_commit_ts catalog is copied.

From 5fcbacd870027afe9d59bec696fadfae5727db5b Mon Sep 17 00:00:00 2001
From: Sergey Levin <[email-masked]>
Date: Thu, 3 Apr 2025 21:44:01 +0500
Subject: [PATCH v1] Migration of the pg_commit_ts directory

---
 src/bin/pg_upgrade/controldata.c | 23 ++++++++++++++++++++++-
 src/bin/pg_upgrade/pg_upgrade.c  |  6 ++++--
 src/bin/pg_upgrade/pg_upgrade.h  |  2 ++
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 47ee27ec835..0da5ee4225e 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -207,7 +207,8 @@ get_control_data(ClusterInfo *cluster)
 		cluster->controldata.data_checksum_version = 0;
 		got_data_checksum_version = true;
 	}
-
+	cluster->controldata.chkpnt_oldstCommitTsxid = 0;
+	cluster->controldata.chkpnt_newstCommitTsxid = 0;
 	/* we have the result of cmd in "output". so parse it line by line now */
 	while (fgets(bufin, sizeof(bufin), output))
 	{
@@ -320,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/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 9295e46aed3..746f8a6b02b 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -751,6 +751,8 @@ copy_xact_xlog_xid(void)
 					  "pg_clog" : "pg_xact",
 					  GET_MAJOR_VERSION(new_cluster.major_version) <= 906 ?
 					  "pg_clog" : "pg_xact");
+	if (old_cluster.controldata.chkpnt_oldstCommitTsxid > 0)
+					   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,
@@ -773,8 +775,8 @@ copy_xact_xlog_xid(void)
 	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,
+			  old_cluster.controldata.chkpnt_oldstCommitTsxid == 0 ? old_cluster.controldata.chkpnt_nxtxid : old_cluster.controldata.chkpnt_oldstCommitTsxid,
+			  old_cluster.controldata.chkpnt_newstCommitTsxid == 0 ? old_cluster.controldata.chkpnt_nxtxid : old_cluster.controldata.chkpnt_newstCommitTsxid,
 			  new_cluster.pgdata);
 	check_ok();
 
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 69c965bb7d0..b55e2c2ea84 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;
-- 
2.42.2

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Mon, 5 Jan 2026 08:24:57 +0000

Dear Hackers,

I found that v7 needs rebased. Copyright was also updated in the attached patch.
I'm not the author of the patch though.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: ls7777 <[email-masked]>
Date: Mon, 05 Jan 2026 16:27:26 +0500

Hi,

I didn't quite understand what I needed to do.

My assumptions:

1. You need to download the postgresql master branch and create a patch file on it.

2. Replace the 2025 with 2026 header comment in all сhangeable patch files.

Did I understand correctly?

----------------

Кому: 'ls7777' ([email-masked]);

Копия: [email-masked], [email-masked], [email-masked];

Тема: Patch for migration of the pg_commit_ts directory;

05.01.2026, 13:25, "Hayato Kuroda (Fujitsu)" <[email-masked]>:

Dear Hackers,

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Tue, 6 Jan 2026 00:41:53 +0000

Hi,

> 1. You need to download the postgresql master branch and create a patch file on it.

This is correct. I periodically checked the commitfest app [1], and it has
reported [Needs rebase!] for weeks. Now it could be applied cleanly and tests
could be passed.

> 2. Replace the 2025 with 2026 header comment in all сhangeable patch files.

Since I cannot follow the sentence, let me clarify my understanding.

Basically, the copyright notation is maintained by the PostgreSQL community;
nothing for us to do. They are updated at the beginning of the year [2].
If you are proposing to add new files, however, they must contain the copyright
and be updated in the new year. It's not yet included in the codebase and is out
of scope for the community's maintenance.

Please ask me anything if you have more questions :-).

[1]: https://commitfest.postgresql.org/patch/6119/ 
[2]: https://github.com/postgres/postgres/commit/451c43974f8e199097d97624a4952ad0973cea61

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: Amit Kapila <[email-masked]>
Date: Fri, 20 Feb 2026 13:28:39 +0530

> I'm not the author of the patch though.

Can we update the use case of this patch in the commit message? One of
the use case I recall is to detect conflicts with accuracy after
upgrade. See docs [1] ("Commit timestamps and origin data are not
preserved during the upgrade. ..) I think this note needs an update
after this patch.

res = executeQueryOrDie(conn, "SELECT setting FROM pg_settings "
+ "WHERE name = 'track_commit_timestamp'");
+ track_commit_timestamp_on = strcmp(PQgetvalue(res, 0, 0), "on") == 0;

As this is a boolean variable, what if the user sets the value of this
GUC as true, will the above work correctly? Also, _on in the variable
name appears bit odd.
-- 
With Regards,
Amit Kapila.

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Fri, 20 Feb 2026 11:05:37 +0000

Dear Amit,

> One of

Right, and code comments [a] should be also updated.

BTW, is there a possibility that we can export and import xmin of the conflict
slot to retain dead tuples even after the upgrade? Of course it's out-of-scope
from this thread.

> GUC as true, will the above work correctly?

Per my understanding, it is OK to use "on"/"off" notation. Below contains the
analysis.

pg_settings uses an SQL function pg_show_all_settings, and it is implemneted by
the C function show_all_settings(). And GetConfigOptionValues()->ShowGUCOption()
actualy create the output for the setting column in the view. According to the
function, the boolean would be the either "on" or "off" unless the GUC has show_hook.

```
	switch (record->vartype)
	{
		case PGC_BOOL:
			{
				const struct config_bool *conf = &record->_bool;

				if (conf->show_hook)
					val = conf->show_hook();
				else
					val = *conf->variable ? "on" : "off";
			}
			break;
```

It mactches my experiment below.

```
$ cat data_pub/postgresql.conf | grep track_commit_timestamp
track_commit_timestamp = true   # collect timestamp of transaction commit
$ psql -U postgres -p 5432
postgres=# SELECT setting FROM pg_settings WHERE name = 'track_commit_timestamp';
 setting 
---------
 on
(1 row)
```

> Also, _on in the variable name appears bit odd.

I have two options, thought?
 - commit_ts_is_enabled,
 - track_commit_timestmap, same as GUC variable.

[a]: https://github.com/postgres/postgres/blob/master/src/bin/pg_upgrade/pg_upgrade.c#L215

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: Amit Kapila <[email-masked]>
Date: Fri, 20 Feb 2026 17:04:52 +0530

> 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.

> slot to retain dead tuples even after the upgrade? Of course it's out-of-scope

Yeah, this is a good point to explore separately. The key thing we
need to evaluate is whether the rows corresponding to old_xids are
retained. We probably need to evaluate the epoch part as well in old
cluster's slot. We do call set_frozenxids() for new cluster that might
have some impact on the functionality you are looking at.

> I have two options, thought?

This looks reasonable to me.

-- 
With Regards,
Amit Kapila.

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Mon, 23 Feb 2026 04:41:30 +0000

Dear Amit,

> 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

---

From: ls7777 <[email-masked]>
Date: Mon, 23 Feb 2026 18:33:01 +0500

Hello.

Thanks for updating the patch. I am using a translator and will not be able to edit the documentation and comments correctly.

----------------

Кому: 'Amit Kapila' ([email-masked]);

Копия: [email-masked], [email-masked];

23.02.2026, 09:41, "Hayato Kuroda (Fujitsu)" <[email-masked]>:

Dear Amit,

0.

```
Table "public.employee"

1.

```

2.

```

3.

4.

```

5.

```

6.

```

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Wed, 27 May 2026 05:24:54 +0000

Dear Sergey,

While considering the feature once again, I came up with another issue.

When pg_upgrade migrates objects to a new instance, it normally checks whether
the new instance does not have existing objects. E.g., check_new_cluster_is_empty()
checks if no tables exist, and check_new_cluster_replication_slots() ensures
there are no logical slots.

Based on that, how should we handle the commit timestamp? Since the patch simply
copies the pg_commit_ts subdir, existing entries in the new cluster will be overwritten.
So we may have to check if the commit_ts entries are empty when the pg_upgrade
command runs, but not sure how to do that.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: ls7777 <[email-masked]>
Date: Fri, 29 May 2026 21:08:56 +0500

Hi,

During copying, the copy_subdir_files function cleans up the target directory with the remove_new_subdir(new_subdir, true) function.

----------------

Копия: [email-masked], [email-masked], [email-masked], Masahiko Sawada ([email-masked]);

27.05.2026, 10:25, "Hayato Kuroda (Fujitsu)" <[email-masked]>:

Dear Sergey,

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Mon, 1 Jun 2026 01:25:31 +0000

Dear Sergey,

> During copying, the copy_subdir_files function cleans up the target
> directory with the remove_new_subdir(new_subdir, true) function.

Correct, the function can discard existing directory. But my point was that how
do we ENSURE it's OK to remove.

Best regards,
Hayato Kuroda
FUJITSU LIMITED