Обсуждение: RE: Patch for migration of the pg_commit_ts directory

Поиск
Список
Период
Сортировка

RE: Patch for migration of the pg_commit_ts directory

От
"Hayato Kuroda (Fujitsu)"
Дата:
Hi,

Per my understanding, track_commit_timestamp must be on the new instance,
otherwise it would be removed when new instance starts. Is it correct?
If so, should we detect the difference and do some warnings/errors here?
Or it is just the user's responsibility?

Best regards,
Hayato Kuroda
FUJITSU LIMITED


RE: Patch for migration of the pg_commit_ts directory

От
"Hayato Kuroda (Fujitsu)"
Дата:
Hi,
(Sorry but I cannot find your name)

> Yes, track_commit_timestamp must be installed in the new instance.
> This is only the responsibility of an experienced user.
> pg_upgrage should allow you to save pg_commit_ts if this data exists at the time of migration.
> Warnings are not needed, the loss of this data is not critical in most cases.
> They were lost with each migration if users did not manually migrate them.

So, your policy is that commit_ts is not the user data thus it is OK to drop during
the upgrade, is it correct? I want to know other's opinion around here.
Note that if we want to check, it can be done in check_control_data().

Regarding the patch:
```
-
+    cluster->controldata.chkpnt_oldstCommitTsxid = 0;
+    cluster->controldata.chkpnt_newstCommitTsxid = 0;
```

Other attributes are not initialized, you can follow.

```
+    if (old_cluster.controldata.chkpnt_oldstCommitTsxid > 0)
+                       copy_subdir_files("pg_commit_ts", "pg_commit_ts");
```

Indent should be fixed. Please run pgindent.

```
-              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,
```

To confirm, is there a possibility that only either of oldest/newest CommitTs exists?

Best regards,
Hayato Kuroda
FUJITSU LIMITED


Re: Patch for migration of the pg_commit_ts directory

От
Amit Kapila
Дата:
On Thu, Oct 2, 2025 at 12:10 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> > Yes, track_commit_timestamp must be installed in the new instance.
> > This is only the responsibility of an experienced user.
> > pg_upgrage should allow you to save pg_commit_ts if this data exists at the time of migration.
> > Warnings are not needed, the loss of this data is not critical in most cases.
> > They were lost with each migration if users did not manually migrate them.
>
> So, your policy is that commit_ts is not the user data thus it is OK to drop during
> the upgrade, is it correct?
>

IIUC, the proposal is that if GUC track_commit_timestamp is enabled on
the new instance then we should copy it, otherwise, we can drop
copying it. Is my understanding correct? I think we can follow what is
done check_new_cluster_replication_slots() for the case when
track_commit_timestamp is not set on the new server. When we try to
copy slots and the wal_level on the new server is minimal, we error
out, so shouldn't we do the same here and error_out if
track_commit_timestamp is not enabled and we have some valid commit_ts
data to copy?

--
With Regards,
Amit Kapila.