Thread
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Update copyright for 2026
- 451c43974f8e 19 (unreleased) cited
-
Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2025-04-03T18:27:32Z
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. -
Re: Patch for migration of the pg_commit_ts directory
Maxim Orlov <orlovmg@gmail.com> — 2025-09-30T10:35:26Z
Yes, the pg_commit_ts directory is not transferred by pg_upgrade. Basically, you can get the exact same result by manually copying the pg_commit_ts directory and running pg_resetwal. And this patch does it automatically. In other words, the patch provides the stated functionality, but consider running pgindet. I'm not sure whether there were any reasons not to make this move from the beginning. The authors' opinions are needed here. And one more thing. In my perspective, having some tests is always a good idea, even if they are not meant to be committed. After briefly examining the patch, I developed this basic test. This is not finalized in any manner, but it is simply an idea. -- Best regards, Maxim Orlov.
-
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-10-01T12:17:50Z
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) <kuroda.hayato@fujitsu.com> — 2025-10-01T12:27:25Z
Dear Maxim, Thanks for considering the test case. To confirm, where should we put the test code? Pp_upgrade has already had 006 test, and commit_ts has not had 005 test. Regarding the test, it is enough to check whether oldest/newest transactions on the old cluster can be tracked. Best regards, Hayato Kuroda FUJITSU LIMITED
-
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2025-10-01T16:25:26Z
<div><div>Hi,</div><div> </div><div>Yes, track_commit_timestamp must be installed in the new instance.</div><div>This is only the responsibility of an experienced user.</div><div>pg_upgrage should allow you to save pg_commit_ts if this data exists at the time of migration.</div></div><div><div>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.</div><div> </div></div><div>----------------</div><div>Кому: 'ls7777' (ls7777@yandex.ru);</div><div>Копия: pgsql-hackers@postgresql.org;</div><div>Тема: Patch for migration of the pg_commit_ts directory;</div><div>01.10.2025, 17:17, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>:</div><blockquote><p>Hi,<br /><br />Per my understanding, track_commit_timestamp must be on the new instance,<br />otherwise it would be removed when new instance starts. Is it correct?<br />If so, should we detect the difference and do some warnings/errors here?<br />Or it is just the user's responsibility?<br /><br />Best regards,<br />Hayato Kuroda<br />FUJITSU LIMITED<br /> </p></blockquote>
-
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-10-02T06:40:08Z
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 <amit.kapila16@gmail.com> — 2025-10-02T08:49:28Z
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.
-
Re: Patch for migration of the pg_commit_ts directory
Maxim Orlov <orlovmg@gmail.com> — 2025-10-06T07:58:55Z
On Thu, 2 Oct 2025 at 11:49, Amit Kapila <amit.kapila16@gmail.com> wrote: > 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? > +1 Sounds reasonable to me. It's better to give an explicit error; otherwise, we would remove data that the user clearly wants to migrate to the new cluster. And deleting data just because the user forgot to specify one parameter in a new cluster looks like a bad joke to me. -- Best regards, Maxim Orlov.
-
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2025-10-06T14:31:52Z
<div><div>Hi,</div><div><div><span style="white-space:pre-wrap">I'll try to return it to the original </span><span style="background-color:#ffffff;color:#1a1a1a;float:none;font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">thread.</span></div><div> </div><div><div>I absolutely agree with you. Data loss during migration is an unacceptable situation. And why it's still happening, I don't understand. But issuing messages will require translating it into other languages and complicate the patch. I'm not a programmer to do it beautifully. And not a beautiful patch is unlikely to be accepted. </div><div>I will do a parameter check and an ERROR message.</div></div><div> </div></div><div> </div><blockquote><div><span style="background-color:#ffffff;color:#1a1a1a;float:none;font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">Hi,</span><br style="background-color:rgb( 255 , 255 , 255 );color:rgb( 26 , 26 , 26 );font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" /><br style="background-color:rgb( 255 , 255 , 255 );color:rgb( 26 , 26 , 26 );font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" /><span style="background-color:#ffffff;color:#1a1a1a;float:none;font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">Could you please reply to the original thread? Otherwise, it is difficult to track later.</span><br style="background-color:rgb( 255 , 255 , 255 );color:rgb( 26 , 26 , 26 );font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" /><span style="background-color:#ffffff;color:#1a1a1a;float:none;font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">Regarding the patch, I think Amit suggested to raise an ERROR in case of parameter</span><br style="background-color:rgb( 255 , 255 , 255 );color:rgb( 26 , 26 , 26 );font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" /><span style="background-color:#ffffff;color:#1a1a1a;float:none;font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">mismatch, and Maxim agreed the point [2]. How do you feel?</span><br style="background-color:rgb( 255 , 255 , 255 );color:rgb( 26 , 26 , 26 );font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" /><br style="background-color:rgb( 255 , 255 , 255 );color:rgb( 26 , 26 , 26 );font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" /><span style="background-color:#ffffff;color:#1a1a1a;float:none;font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">[1]: </span><a href="https://www.postgresql.org/message-id/CAA4eK1%2BZayox%3Dx84upet_gc4hrCKSwZSpnO9%3DQ4vH8rrJbsBOg%40mail.gmail.com" rel="noopener noreferrer" target="_blank" style="background-color:rgb( 255 , 255 , 255 );color:rgb( 0 , 119 , 255 );font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">https://www.postgresql.org/message-id/CAA4eK1%2BZayox%3Dx84upet_gc4hrCKSwZSpnO9%3DQ4vH8rrJbsBOg%40mail.gmail.com</a><br style="background-color:rgb( 255 , 255 , 255 );color:rgb( 26 , 26 , 26 );font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" /><span style="background-color:#ffffff;color:#1a1a1a;float:none;font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">[2]: </span><a href="https://www.postgresql.org/message-id/CACG%3DezaFdL4EM8bdrHTRVHt5BmxxWGXcPiDr6Qm00Qj%2BS_Y%2BvQ%40mail.gmail.com" rel="noopener noreferrer" target="_blank" style="background-color:rgb( 255 , 255 , 255 );color:rgb( 0 , 119 , 255 );font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">https://www.postgresql.org/message-id/CACG%3DezaFdL4EM8bdrHTRVHt5BmxxWGXcPiDr6Qm00Qj%2BS_Y%2BvQ%40mail.gmail.com</a><br style="background-color:rgb( 255 , 255 , 255 );color:rgb( 26 , 26 , 26 );font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" /><br style="background-color:rgb( 255 , 255 , 255 );color:rgb( 26 , 26 , 26 );font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" /><span style="background-color:#ffffff;color:#1a1a1a;float:none;font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">Best regards,</span><br style="background-color:rgb( 255 , 255 , 255 );color:rgb( 26 , 26 , 26 );font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" /><span style="background-color:#ffffff;color:#1a1a1a;float:none;font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">Hayato Kuroda</span><br style="background-color:rgb( 255 , 255 , 255 );color:rgb( 26 , 26 , 26 );font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" /><span style="background-color:#ffffff;color:#1a1a1a;float:none;font-family:'ys text' , 'arial' , sans-serif;font-size:16px;font-style:normal;font-weight:400;text-align:start;text-decoration-color:initial;text-decoration-style:initial;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">FUJITSU LIMITED</span></div></blockquote></div><div> </div><div>----------------</div><div>Кому: Amit Kapila (amit.kapila16@gmail.com);</div><div>Копия: Hayato Kuroda (Fujitsu) (kuroda.hayato@fujitsu.com), pgsql-hackers@postgresql.org;</div><div>Тема: Patch for migration of the pg_commit_ts directory;</div><div>06.10.2025, 12:59, "Maxim Orlov" <orlovmg@gmail.com>:</div><blockquote><div><div> </div> <div><div>On Thu, 2 Oct 2025 at 11:49, Amit Kapila <<a href="mailto:amit.kapila16@gmail.com" rel="noopener noreferrer">amit.kapila16@gmail.com</a>> wrote:</div><blockquote style="border-left-color:rgb( 204 , 204 , 204 );border-left-style:solid;border-left-width:1px;margin:0px 0px 0px 0.8ex;padding-left:1ex"> When we try to<br />copy slots and the wal_level on the new server is minimal, we error<br />out, so shouldn't we do the same here and error_out if<br />track_commit_timestamp is not enabled and we have some valid commit_ts<br />data to copy?</blockquote><div> </div><div>+1 Sounds reasonable to me. It's better to give an explicit error;<br />otherwise, we would remove data that the user clearly wants to migrate<br />to the new cluster. And deleting data just because the user forgot to<br />specify one parameter in a new cluster looks like a bad joke to me. </div><div> </div></div>-- <div><div><div>Best regards,</div><div>Maxim Orlov.</div></div></div></div></blockquote>
-
Re: Patch for migration of the pg_commit_ts directory
Bruce Momjian <bruce@momjian.us> — 2025-10-06T15:45:15Z
On Mon, Oct 6, 2025 at 07:31:52PM +0500, ls7777 wrote: > Hi, > I'll try to return it to the original thread. > > I absolutely agree with you. Data loss during migration is an unacceptable > situation. And why it's still happening, I don't understand. But issuing > messages will require translating it into other languages and complicate the > patch. I'm not a programmer to do it beautifully. And not a beautiful patch is > unlikely to be accepted. > I will do a parameter check and an ERROR message. You don't need to worry about translations. Each language team will make sure your English message is translated. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com Do not let urgent matters crowd out time for investment in the future.
-
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2025-10-06T17:30:29Z
Hi. Added a fatal message about an incorrect track_commit_timestamp parameter for new cluster. Update test. Fail check: Checking for new cluster tablespace directories ok Checking for pg_commit_ts "track_commit_timestamp" must be "on" but is set to "off" Successful: Checking for new cluster tablespace directories ok Checking for pg_commit_ts ok ... ---------------- Кому: Amit Kapila (amit.kapila16@gmail.com); Копия: Hayato Kuroda (Fujitsu) (kuroda.hayato@fujitsu.com), pgsql-hackers@postgresql.org; Тема: Patch for migration of the pg_commit_ts directory; 06.10.2025, 12:59, "Maxim Orlov" <orlovmg@gmail.com>: On Thu, 2 Oct 2025 at 11:49, Amit Kapila <amit.kapila16@gmail.com> wrote: 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? +1 Sounds reasonable to me. It's better to give an explicit error; otherwise, we would remove data that the user clearly wants to migrate to the new cluster. And deleting data just because the user forgot to specify one parameter in a new cluster looks like a bad joke to me. -- Best regards, Maxim Orlov. -
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-10-07T03:15:18Z
Hi, Thanks for updating the patch. Regarding the check_track_commit_timestamp_parameter(), I'm not sure the function is needed. Since pg_controldata outputs whether the commit_ts module is enabled or not [1], can we obtain there instead? I imagined like we can add a new field at ControlData, it could be obtained in get_control_data(), values for instances could be compared in check_control_data(). Another possibility is that oldestCommitTsXid/newestCommitTsXid can be always valid if track_commit_timestamp is on. In this case the attribute is not needed anymore. It is enough to check these values. Can you check them? Regarding the test, I feel it is unnecessary long. I feel it is enough to ensure one or two transactions have the same timestamp on the both nodes, pgbench is not needed. Current test may not be able to finish on the slow machine. [1] ``` $ pg_controldata data/ | grep "track_" track_commit_timestamp setting: off ``` Best regards, Hayato Kuroda FUJITSU LIMITED
-
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2025-10-07T04:25:37Z
<div>Hi,</div><div> </div><div><div>At the time check_control_data is launched for the new cluster, the control file does not contain information about the set track_commit_timestamp=on. It is installed only in postgresql.conf. The check_track_commit_timestamp_parameter function is only needed for the new cluster. And you can not run it for the old cluster, but use data from the old_cluster.controldata.chkpnt_newstCommitTsxid control file. But this will be less clear than using the check_track_commit_timestamp_parameter function.</div><div> </div><div>The test can be shortened.</div></div><div> </div><div>----------------</div><div>Кому: 'ls7777' (ls7777@yandex.ru), orlovmg@gmail.com (orlovmg@gmail.com), amit.kapila16@gmail.com (amit.kapila16@gmail.com);</div><div>Копия: pgsql-hackers@postgresql.org;</div><div>Тема: Patch for migration of the pg_commit_ts directory;</div><div>07.10.2025, 08:15, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>:</div><blockquote><p>Hi,<br /><br />Thanks for updating the patch.<br /><br />Regarding the check_track_commit_timestamp_parameter(), I'm not sure the function<br />is needed. Since pg_controldata outputs whether the commit_ts module is enabled<br />or not [1], can we obtain there instead?<br />I imagined like we can add a new field at ControlData, it could be obtained in get_control_data(),<br />values for instances could be compared in check_control_data().<br /><br />Another possibility is that oldestCommitTsXid/newestCommitTsXid can be always<br />valid if track_commit_timestamp is on. In this case the attribute is not needed<br />anymore. It is enough to check these values. Can you check them?<br /><br />Regarding the test, I feel it is unnecessary long. I feel it is enough to ensure<br />one or two transactions have the same timestamp on the both nodes, pgbench is not<br />needed. Current test may not be able to finish on the slow machine.<br /><br />[1]<br />```<br />$ pg_controldata data/ | grep "track_"<br />track_commit_timestamp setting: off<br />```<br /><br />Best regards,<br />Hayato Kuroda<br />FUJITSU LIMITED<br /> </p></blockquote>
-
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-10-07T05:57:14Z
Hi, > At the time check_control_data is launched for the new cluster, the control file > does not contain information about the set track_commit_timestamp=on. It is > installed only in postgresql.conf. You did consider the case that track_commit_timestamp is set to on but the instance has not started before the pg_upgrade, right? Valid point. > The check_track_commit_timestamp_parameter function is only needed for the new > cluster. And you can not run it for the old cluster, but use data from the > old_cluster.controldata.chkpnt_newstCommitTsxid control file. But this will be > less clear than using the check_track_commit_timestamp_parameter function. Sorry, I'm not very clear this part. Can you describe the point bit more? Other comments: 01. get_control_data ``` + if (GET_MAJOR_VERSION(cluster->major_version) < 905) + { + cluster->controldata.chkpnt_oldstCommitTsxid = 0; + cluster->controldata.chkpnt_newstCommitTsxid = 0; + } ``` IIUC, checksum_version is checked like this because got_data_checksum_version must be also set. Since we do not have the boolean for commit_ts, not sure it is needed. old_cluster and new_cluster are the global variable and they are initialized with zero. 02. check_track_commit_timestamp_parameter ``` + conn = connectToServer(cluster, "template1"); + res = executeQueryOrDie(conn, "SELECT count(*) AS is_set " + "FROM pg_settings WHERE name = 'track_commit_timestamp' and setting = 'on'"); + is_set = PQfnumber(res, "is_set"); + cluster->track_commit_timestamp_on = atoi(PQgetvalue(res, 0, is_set)) == 1; ``` The SQL looks strange for me. Isn't it enough to directly obtain "setting" attribute and check it is "on" or not? Also, conn and res can be the local variable of the else part. 03. ClusterInfo ``` + bool track_commit_timestamp_on; /* track_commit_timestamp for + * cluster */ ``` The indent is broken, please run pgindent. 04. test Could you please update meson.build? Otherwise the added test could not be run for meson build. Best regards, Hayato Kuroda FUJITSU LIMITED -
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2025-10-07T06:52:03Z
Hi, You did consider the case that track_commit_timestamp is set to on but the instance has not started before the pg_upgrade, right? Valid point. Yes. Sorry, I'm not very clear this part. Can you describe the point bit more? Easy-to-read code. But I've already changed everything. I've followed all the recommendations. Removed the function. Shortened the test. 01. Removed initialization of variables. 02. Change 03. Removed 04. Add ---------------- Кому: 'ls7777' (ls7777@yandex.ru), orlovmg@gmail.com (orlovmg@gmail.com), amit.kapila16@gmail.com (amit.kapila16@gmail.com); Копия: pgsql-hackers@postgresql.org; Тема: Patch for migration of the pg_commit_ts directory; 07.10.2025, 10:57, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>: Hi, At the time check_control_data is launched for the new cluster, the control file does not contain information about the set track_commit_timestamp=on. It is installed only in postgresql.conf. You did consider the case that track_commit_timestamp is set to on but the instance has not started before the pg_upgrade, right? Valid point. The check_track_commit_timestamp_parameter function is only needed for the new cluster. And you can not run it for the old cluster, but use data from the old_cluster.controldata.chkpnt_newstCommitTsxid control file. But this will be less clear than using the check_track_commit_timestamp_parameter function. Sorry, I'm not very clear this part. Can you describe the point bit more? Other comments: 01. get_control_data ``` + if (GET_MAJOR_VERSION(cluster->major_version) < 905) + { + cluster->controldata.chkpnt_oldstCommitTsxid = 0; + cluster->controldata.chkpnt_newstCommitTsxid = 0; + } ``` IIUC, checksum_version is checked like this because got_data_checksum_version must be also set. Since we do not have the boolean for commit_ts, not sure it is needed. old_cluster and new_cluster are the global variable and they are initialized with zero. 02. check_track_commit_timestamp_parameter ``` + conn = connectToServer(cluster, "template1"); + res = executeQueryOrDie(conn, "SELECT count(*) AS is_set " + "FROM pg_settings WHERE name = 'track_commit_timestamp' and setting = 'on'"); + is_set = PQfnumber(res, "is_set"); + cluster->track_commit_timestamp_on = atoi(PQgetvalue(res, 0, is_set)) == 1; ``` The SQL looks strange for me. Isn't it enough to directly obtain "setting" attribute and check it is "on" or not? Also, conn and res can be the local variable of the else part. 03. ClusterInfo ``` + bool track_commit_timestamp_on; /* track_commit_timestamp for + * cluster */ ``` The indent is broken, please run pgindent. 04. test Could you please update meson.build? Otherwise the added test could not be run for meson build. Best regards, Hayato Kuroda FUJITSU LIMITED -
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-10-09T06:22:26Z
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 -
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2025-10-09T10:24:33Z
Hi, Followed all recommendations. ---------------- Кому: 'ls7777' (ls7777@yandex.ru); Копия: pgsql-hackers@postgresql.org, orlovmg@gmail.com, amit.kapila16@gmail.com; Тема: Patch for migration of the pg_commit_ts directory; 09.10.2025, 11:22, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>: 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 -
Re: Patch for migration of the pg_commit_ts directory
Maxim Orlov <orlovmg@gmail.com> — 2025-10-10T10:55:34Z
Looks good to me. As I see it, the patch is almost ready, so I decided to create an CF entry [0]. Feel free to edit it as you see fit. And your mail also includes attachment (unknown_filename <https://www.postgresql.org/message-id/attachment/175614/unknown_filename> ), interfering with the CF-bot. [0] https://commitfest.postgresql.org/patch/6119/
-
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2025-10-10T14:19:58Z
<div><div>Thanks.</div><div>That's the good news.</div><div><div><div><span style="white-space:pre-wrap">Thanks to Hayato Kuroda, for the advice.</span></div><div><span style="white-space:pre-wrap">Thanks to everyone who participated in the promotion of the patch.</span></div><div> </div></div></div></div><div> </div><div>----------------</div><div>Кому: ls7777 (ls7777@yandex.ru);</div><div>Копия: Hayato Kuroda (Fujitsu) (kuroda.hayato@fujitsu.com), pgsql-hackers@postgresql.org, amit.kapila16@gmail.com;</div><div>Тема: Patch for migration of the pg_commit_ts directory;</div><div>10.10.2025, 15:55, "Maxim Orlov" <orlovmg@gmail.com>:</div><blockquote><div><div><span lang="en">Looks good to me. As I see it, the patch is almost ready, so I decided </span></div><div><span lang="en">to create an CF entry [0]. Feel free to edit it as you see fit.</span></div><div> </div><div><span lang="en">And your mail also includes a</span>ttachment (<a href="https://www.postgresql.org/message-id/attachment/175614/unknown_filename" rel="noopener noreferrer">unknown_filename</a>), <span lang="en">interfering with the CF-bot.</span></div><div> </div><div>[0] <a href="https://commitfest.postgresql.org/patch/6119/" rel="noopener noreferrer">https://commitfest.postgresql.org/patch/6119/</a></div></div></blockquote>
-
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-10-14T04:27:23Z
Hi, Thanks for updating the patch. ``` +my ($xid,$ts) = $resold =~ /\s*(\d+)\s*\|(.*)/; ``` Per my understanding $ts is not used here. Can you remove it? Apart from above, I found some cosmetic issues. Please see attached my fix which can be applied atop HEAD. Can you check and include if it is OK? Best regards, Hayato Kuroda FUJITSU LIMITED
-
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2025-10-14T15:53:04Z
Hi, I applied the diff file. Deleted the $ts variable. ---------------- Кому: 'ls7777' (ls7777@yandex.ru); Копия: pgsql-hackers@postgresql.org, orlovmg@gmail.com, amit.kapila16@gmail.com; Тема: Patch for migration of the pg_commit_ts directory; 14.10.2025, 09:27, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>: Hi, Thanks for updating the patch. ``` +my ($xid,$ts) = $resold =~ /\s*(\d+)\s*\|(.*)/; ``` Per my understanding $ts is not used here. Can you remove it? Apart from above, I found some cosmetic issues. Please see attached my fix which can be applied atop HEAD. Can you check and include if it is OK? Best regards, Hayato Kuroda FUJITSU LIMITED -
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-10-15T01:46:23Z
Hi, Thanks for updating the patch. Mostly looks good. I ran pgindent locally and indents of my part were incorrect. PSA the included version. I have no comments anymore. Thanks for the great work. Best regards, Hayato Kuroda FUJITSU LIMITED
-
Re: Patch for migration of the pg_commit_ts directory
Maxim Orlov <orlovmg@gmail.com> — 2025-10-15T05:12:32Z
On Wed, 15 Oct 2025 at 04:46, Hayato Kuroda (Fujitsu) < kuroda.hayato@fujitsu.com> wrote: > Hi, > > Thanks for updating the patch. Mostly looks good. Yep, looks good to me too. If there are no objections, I will move forward to mark the thread as ready for committers. Hayato Kuroda, you did a great job reviewing this patch; consider to tag yourself as a reviewer [0]. [0] https://commitfest.postgresql.org/patch/6119/ -- Best regards, Maxim Orlov.
-
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-01-05T08:24:57Z
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
-
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2026-01-05T11:27:26Z
<div><div>Hi,</div><div> </div><div><div><div>I didn't quite understand what I needed to do. </div><div>My assumptions:</div><div>1. You need to download the postgresql master branch and create a patch file on it.</div><div> </div><div>2. Replace the 2025 with 2026 header comment in all с<span style="white-space:pre-wrap">hangeable</span> patch files.</div><div> </div><div>Did I understand correctly?</div></div></div></div><div> </div><div>----------------</div><div>Кому: 'ls7777' (ls7777@yandex.ru);</div><div>Копия: pgsql-hackers@postgresql.org, orlovmg@gmail.com, amit.kapila16@gmail.com;</div><div>Тема: Patch for migration of the pg_commit_ts directory;</div><div>05.01.2026, 13:25, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>:</div><blockquote><p>Dear Hackers,<br /><br />I found that v7 needs rebased. Copyright was also updated in the attached patch.<br />I'm not the author of the patch though.<br /><br />Best regards,<br />Hayato Kuroda<br />FUJITSU LIMITED<br /> </p></blockquote>
-
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-01-06T00:41:53Z
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. 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
-
Re: Patch for migration of the pg_commit_ts directory
Amit Kapila <amit.kapila16@gmail.com> — 2026-02-20T07:58:39Z
On Mon, Jan 5, 2026 at 1:54 PM Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> wrote: > > I found that v7 needs rebased. Copyright was also updated in the attached patch. > 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. -
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-02-20T11:05:37Z
Dear Amit, > 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. 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. > 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? 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 -
Re: Patch for migration of the pg_commit_ts directory
Amit Kapila <amit.kapila16@gmail.com> — 2026-02-20T11:34:52Z
On Fri, Feb 20, 2026 at 4:35 PM Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> wrote: > > > 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. > > 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. > 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. > 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. > > Also, _on in the variable name appears bit odd. > > I have two options, thought? > - commit_ts_is_enabled, This looks reasonable to me. -- With Regards, Amit Kapila. -
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-02-23T04:41:30Z
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 -
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2026-02-23T13:33:01Z
<div><div>Hello.</div><div> </div><div>Thanks for updating the patch. I am using a translator and will not be able to edit the documentation and comments correctly.</div><div> </div></div><div> </div><div>----------------</div><div>Кому: 'Amit Kapila' (amit.kapila16@gmail.com);</div><div>Копия: pgsql-hackers@postgresql.org, orlovmg@gmail.com;</div><div>Тема: Patch for migration of the pg_commit_ts directory;</div><div>23.02.2026, 09:41, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>:</div><blockquote><p>Dear Amit,<br /> </p><blockquote> > Right, and code comments [a] should be also updated.<br /> ><br /> <br /> So, leaving aside update_delete, copying commit_ts could be helpful to<br /> detect some other conflicts. You may want to once test the same and<br /> show it here as part of use case establishment.</blockquote><p><br />I confirmed that {update|delete}_origin_differs could be detected with the<br />following steps.<br /><br />0.<br />Constructed pub-sub replication system. track_commit_timestamp=on was set on the<br />subscriber, and the table below was defined on both clusters<br /><br />```<br /> Table "public.employee"<br /> Column | Type | Collation | Nullable | Default<br />--------+---------+-----------+----------+---------<br /> id | integer | | not null |<br /> salary | integer | | |<br />Indexes:<br /> "employee_pkey" PRIMARY KEY, btree (id)<br />```<br /><br /><br />1.<br />Inserted a tuple on the publisher<br /><br />```<br />pub=# INSERT INTO employee VALUES (1, 100);<br />INSERT 0 1<br />```<br /><br />2.<br />UPDATEd the replicated tuple on the subscriber. Confirmed that commit timestamps<br />were stored.<br /><br />```<br />sub=# SELECT * FROM pg_last_committed_xact();<br /> xid | timestamp | roident<br />-----+-------------------------------+---------<br /> 738 | 2026-02-23 13:17:19.263146+09 | 1<br />(1 row)<br />sub=# UPDATE employee SET salary = 10 WHERE id = 1;<br />UPDATE 1<br />sub=# SELECT * FROM pg_last_committed_xact();<br /> xid | timestamp | roident<br />-----+-------------------------------+---------<br /> 739 | 2026-02-23 13:17:33.230773+09 | 0<br />(1 row)<br />```<br /><br />3.<br />Ran pg_upgrade to upgrade the subscriber. The new cluster must also set<br />track_commit_timestamp to on.<br /><br />4.<br />Confirmed commit timestamps could be migrated.<br /><br />```<br />new=# SELECT * FROM pg_xact_commit_timestamp_origin('739');<br /> timestamp | roident<br />-------------------------------+---------<br /> 2026-02-23 13:17:33.230773+09 | 0<br />(1 row)<br />```<br /><br />5.<br />UPDATEd the tuple on the publisher.<br /><br />```<br />pub=# UPDATE employee SET salary = 200 WHERE id = 1;<br />UPDATE 1<br />```<br /><br />6.<br />update_origin_differs conflict was detected on the new subscriber.<br /><br />```<br />LOG: conflict detected on relation "public.employee": conflict=update_origin_differs<br />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).<br />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<br />```<br /><br />One debatable point is whether we should include this in the TAP test. Personally<br />It's not necessary to simplify the test; either one is OK.<br /><br /><br />Not sure it is OK, but I created updated patches and considered a commit message.<br />0001 was not changed from the original, and 0002 addressed comments from you and<br />contained the commit message. For now I added my name as one of the author, but<br />OK to be listed as reviewer. Most of parts were written by Sergey.<br /><br />Best regards,<br />Hayato Kuroda<br />FUJITSU LIMITED<br /> </p></blockquote> -
Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2026-03-05T05:04:05Z
- amit.kapila16@ Hello, Needs rebase https://commitfest.postgresql.org/patch/6119/ The patch version, taking into account all changes and recommendations. -
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-03-10T02:50:49Z
Dear Sergey, You seemed to merge the patch correctly, but missing update the commit message. Best regards, Hayato Kuroda FUJITSU LIMITED
-
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2026-03-10T14:39:59Z
<div>Hello,</div><div> </div><div><div>I don't understand what I need to do.</div></div><div> </div><div><blockquote><div>Subject: [PATCH v10] Migration of the pg_commit_ts directory</div></blockquote></div><div> </div><div><div>I have specified a new version in the patch. Do I need to add anything else?</div></div><div> </div><div>----------------</div><div>Кому: 'ls7777' (ls7777@yandex.ru);</div><div>Копия: pgsql-hackers@postgresql.org, orlovmg@gmail.com;</div><div>Тема: Patch for migration of the pg_commit_ts directory;</div><div>10.03.2026, 07:50, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>:</div><blockquote><p>Dear Sergey,<br /><br />You seemed to merge the patch correctly, but missing update the commit message.<br /><br />Best regards,<br />Hayato Kuroda<br />FUJITSU LIMITED<br /> </p></blockquote>
-
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-03-11T07:55:36Z
Dear Sergey, > I don't understand what I need to do. >Subject: [PATCH v10] Migration of the pg_commit_ts directory > I have specified a new version in the patch. Do I need to add anything else? When we see the commit message in postgres repo, you can see that each commit contain the background, what we did in the commit, considerations, and etc. Compared with them, the description you added seems insufficient. Of course the committer will take care, but Amit suggested [1] to write a draft. The basic style guide is determined in [2]. Actually I wrote a draft for the patch set in v9-0002, but you missed. You may able to refer it, and re-create new version or use the same one. [1]: https://www.postgresql.org/message-id/CAA4eK1KMuhaH%3DQgzJCwQgawuybosq-JMWWT9qSqyMZNV1MWjtg%40mail.gmail.com [2]: https://wiki.postgresql.org/wiki/Commit_Message_Guidance Best regards, Hayato Kuroda FUJITSU LIMITED
-
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2026-03-11T17:51:11Z
Hello, I saw a big comment that I had missed. Added fixes in two patches: v11-0001-Migration-of-the-pg_commit_ts-directory.patch - main changes v11-0002-Migration-of-the-pg_commit_ts-directory-desc.patch - changes to documentation and comments ---------------- Кому: 'ls7777' (ls7777@yandex.ru); Копия: pgsql-hackers@postgresql.org, orlovmg@gmail.com; Тема: Patch for migration of the pg_commit_ts directory; 11.03.2026, 12:55, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>: Dear Sergey, I don't understand what I need to do. Subject: [PATCH v10] Migration of the pg_commit_ts directory I have specified a new version in the patch. Do I need to add anything else? When we see the commit message in postgres repo, you can see that each commit contain the background, what we did in the commit, considerations, and etc. Compared with them, the description you added seems insufficient. Of course the committer will take care, but Amit suggested [1] to write a draft. The basic style guide is determined in [2]. Actually I wrote a draft for the patch set in v9-0002, but you missed. You may able to refer it, and re-create new version or use the same one. [1]: https://www.postgresql.org/message-id/CAA4eK1KMuhaH%3DQgzJCwQgawuybosq-JMWWT9qSqyMZNV1MWjtg%40mail.gmail.com [2]: https://wiki.postgresql.org/wiki/Commit_Message_Guidance Best regards, Hayato Kuroda FUJITSU LIMITED -
Re: Patch for migration of the pg_commit_ts directory
Masahiko Sawada <sawada.mshk@gmail.com> — 2026-03-16T21:03:39Z
Hi, On Wed, Mar 11, 2026 at 10:51 AM ls7777 <ls7777@yandex.ru> wrote: > > Hello, > I saw a big comment that I had missed. > Added fixes in two patches: > v11-0001-Migration-of-the-pg_commit_ts-directory.patch - main changes > v11-0002-Migration-of-the-pg_commit_ts-directory-desc.patch - changes to documentation and comments > > > ---------------- > Кому: 'ls7777' (ls7777@yandex.ru); > Копия: pgsql-hackers@postgresql.org, orlovmg@gmail.com; > Тема: Patch for migration of the pg_commit_ts directory; > 11.03.2026, 12:55, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>: > > Dear Sergey, > > > I don't understand what I need to do. > Subject: [PATCH v10] Migration of the pg_commit_ts directory > I have specified a new version in the patch. Do I need to add anything else? > > > When we see the commit message in postgres repo, you can see that each commit > contain the background, what we did in the commit, considerations, and etc. > Compared with them, the description you added seems insufficient. > Of course the committer will take care, but Amit suggested [1] to write a draft. > The basic style guide is determined in [2]. > > Actually I wrote a draft for the patch set in v9-0002, but you miss. You may > able to refer it, and re-create new version or use the same one. > I've reviewed this patch and have one question. The commit timestamp has not only the timestamp data but also replication origin id. The replication origin id assigned to the subscription on the new cluster is not necessarily the same as the ones assigned on the old cluster. Therefore, if there is mismatch, replication conflict 'update_origin_differs' could wrongly happen, even though the new subscriber starts logical replication from the same publisher. I've confirmed this behavior in the following scenario: 1. Setup publication. -- on publisher CREATE TABLE test (id int primary key, t text); CREATE PUBLICATION pub FOR TABLE test; 2. Setup subscription. -- on subscriber CREATE TABLE test (id int primary key, t text); CREATE SUBSCRIPTION mysub CONNECTION '...' PUBLICATION pub; CREATE SUBSCRIPTION a_dummy CONNECTION '...' PUBLICATION pub WITH (connect = false, enabled = false); -- confirm that mysub's origin id is 1 and a_dummy's origin id is 2. SELECT oid, subname FROM pg_subscription; oid | subname -------+--------- 16392 | mysub 16393 | a_dummy (2 rows) SELECT * FROM pg_replication_origin; roident | roname ---------+---------- 1 | pg_16392 2 | pg_16393 (2 rows) 3. Insert a tuple to the test table and wait for it to be replicated. -- on publisher INSERT INTO test VALUES (1, 'replicated change'); The change is applied on the subscription with replication origin id 1: -- on subscriber SELECT * FROM test, pg_xact_commit_timestamp_origin(test.xmin); id | c | timestamp | roident ----+-------------------+-------------------------------+--------- 1 | replicated change | 2026-03-16 13:49:35.211316-07 | 1 (1 row) 4. Do pg_upgrade 5. Check the replication origins in the new subscriber SELECT oid, subname FROM pg_subscription; oid | subname -------+--------- 16402 | a_dummy 16403 | mysub (2 rows) SELECT * FROM pg_replication_origin; roident | roname ---------+---------- 1 | pg_16402 2 | pg_16403 (2 rows) The replication origin 1 is now used by a_dummy subscription. 6. Update the tuple on the publisher -- on publisher UPDATE test SET c = 'change after upgrade' WHERE id = 1; Then I got the following LOG: LOG: conflict detected on relation "public.test": conflict=update_origin_differs 2026-03-16 13:53:41.992 PDT [1247879] DETAIL: Updating the row that was modified by a different origin "pg_16402" in transaction 760 at 2026-03-16 13:53:33.448002-07: local row (1, replicated change), remote row (1, change after upgrade), replica identity (id)=(1). 2026-03-16 13:53:41.992 PDT [1247879] CONTEXT: processing remote data for replication origin "pg_16403" during message type "UPDATE" for replication target relation "public.test" in transaction 664, finished at 0/017ED910 I think that this patch works fine only when users don't use the replication origins. On the other hand, I think we should ensure that subscriptions use the same replication origins even after upgrade in cases where users are upgrading subscribers. Also, regarding the patch, with the following change we disallow the case where users want to disable the commit timestamp in the new cluster. IIUC migrating the commit timestamps is not mandatory in practice, so I think we do the migration when applicable or raise a warning if only the new cluster disables the commit timestamps. + if (!commit_ts_is_enabled && + old_cluster.controldata.chkpnt_newstCommitTsxid > 0) + pg_fatal("\"track_commit_timestamp\" must be \"on\" but is set to \"off\""); + Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com -
Re: Patch for migration of the pg_commit_ts directory
Amit Kapila <amit.kapila16@gmail.com> — 2026-03-17T04:16:50Z
On Tue, Mar 17, 2026 at 2:34 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > I've reviewed this patch and have one question. > > The commit timestamp has not only the timestamp data but also > replication origin id. The replication origin id assigned to the > subscription on the new cluster is not necessarily the same as the > ones assigned on the old cluster. Therefore, if there is mismatch, > replication conflict 'update_origin_differs' could wrongly happen, > even though the new subscriber starts logical replication from the > same publisher. I've confirmed this behavior in the following > scenario: > > 1. Setup publication. > -- on publisher > CREATE TABLE test (id int primary key, t text); > CREATE PUBLICATION pub FOR TABLE test; > > 2. Setup subscription. > -- on subscriber > CREATE TABLE test (id int primary key, t text); > CREATE SUBSCRIPTION mysub CONNECTION '...' PUBLICATION pub; > CREATE SUBSCRIPTION a_dummy CONNECTION '...' PUBLICATION pub WITH > (connect = false, enabled = false); > > -- confirm that mysub's origin id is 1 and a_dummy's origin id is 2. > SELECT oid, subname FROM pg_subscription; > oid | subname > -------+--------- > 16392 | mysub > 16393 | a_dummy > (2 rows) > > SELECT * FROM pg_replication_origin; > roident | roname > ---------+---------- > 1 | pg_16392 > 2 | pg_16393 > (2 rows) > > 3. Insert a tuple to the test table and wait for it to be replicated. > -- on publisher > INSERT INTO test VALUES (1, 'replicated change'); > > The change is applied on the subscription with replication origin id 1: > -- on subscriber > SELECT * FROM test, pg_xact_commit_timestamp_origin(test.xmin); > id | c | timestamp | roident > ----+-------------------+-------------------------------+--------- > 1 | replicated change | 2026-03-16 13:49:35.211316-07 | 1 > (1 row) > > 4. Do pg_upgrade > 5. Check the replication origins in the new subscriber > > SELECT oid, subname FROM pg_subscription; > oid | subname > -------+--------- > 16402 | a_dummy > 16403 | mysub > (2 rows) > > SELECT * FROM pg_replication_origin; > roident | roname > ---------+---------- > 1 | pg_16402 > 2 | pg_16403 > (2 rows) > > The replication origin 1 is now used by a_dummy subscription. > > 6. Update the tuple on the publisher > -- on publisher > UPDATE test SET c = 'change after upgrade' WHERE id = 1; > > Then I got the following LOG: > > LOG: conflict detected on relation "public.test": > conflict=update_origin_differs > 2026-03-16 13:53:41.992 PDT [1247879] DETAIL: Updating the row that > was modified by a different origin "pg_16402" in transaction 760 at > 2026-03-16 13:53:33.448002-07: local row (1, replicated change), > remote row (1, change after upgrade), replica identity (id)=(1). > 2026-03-16 13:53:41.992 PDT [1247879] CONTEXT: processing remote data > for replication origin "pg_16403" during message type "UPDATE" for > replication target relation "public.test" in transaction 664, finished > at 0/017ED910 > > I think that this patch works fine only when users don't use the > replication origins. > But is there any value of this patch without replication origins? I thought the value of this patch is to detect conflicts correctly after upgrade. IIUC, in some cases, the commit time on subscriber could still be later than the conflicting transaction from publisher in which case timing data can also be used for last_write_win kind of strategy but without origin that also won't be possible. > On the other hand, I think we should ensure that > subscriptions use the same replication origins even after upgrade in > cases where users are upgrading subscribers. > Right, I think we need to ensure that for detecting conflicts. -- With Regards, Amit Kapila.
-
Re: Patch for migration of the pg_commit_ts directory
Masahiko Sawada <sawada.mshk@gmail.com> — 2026-03-17T17:39:40Z
On Mon, Mar 16, 2026 at 9:17 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Tue, Mar 17, 2026 at 2:34 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > > > I've reviewed this patch and have one question. > > > > The commit timestamp has not only the timestamp data but also > > replication origin id. The replication origin id assigned to the > > subscription on the new cluster is not necessarily the same as the > > ones assigned on the old cluster. Therefore, if there is mismatch, > > replication conflict 'update_origin_differs' could wrongly happen, > > even though the new subscriber starts logical replication from the > > same publisher. I've confirmed this behavior in the following > > scenario: > > > > 1. Setup publication. > > -- on publisher > > CREATE TABLE test (id int primary key, t text); > > CREATE PUBLICATION pub FOR TABLE test; > > > > 2. Setup subscription. > > -- on subscriber > > CREATE TABLE test (id int primary key, t text); > > CREATE SUBSCRIPTION mysub CONNECTION '...' PUBLICATION pub; > > CREATE SUBSCRIPTION a_dummy CONNECTION '...' PUBLICATION pub WITH > > (connect = false, enabled = false); > > > > -- confirm that mysub's origin id is 1 and a_dummy's origin id is 2. > > SELECT oid, subname FROM pg_subscription; > > oid | subname > > -------+--------- > > 16392 | mysub > > 16393 | a_dummy > > (2 rows) > > > > SELECT * FROM pg_replication_origin; > > roident | roname > > ---------+---------- > > 1 | pg_16392 > > 2 | pg_16393 > > (2 rows) > > > > 3. Insert a tuple to the test table and wait for it to be replicated. > > -- on publisher > > INSERT INTO test VALUES (1, 'replicated change'); > > > > The change is applied on the subscription with replication origin id 1: > > -- on subscriber > > SELECT * FROM test, pg_xact_commit_timestamp_origin(test.xmin); > > id | c | timestamp | roident > > ----+-------------------+-------------------------------+--------- > > 1 | replicated change | 2026-03-16 13:49:35.211316-07 | 1 > > (1 row) > > > > 4. Do pg_upgrade > > 5. Check the replication origins in the new subscriber > > > > SELECT oid, subname FROM pg_subscription; > > oid | subname > > -------+--------- > > 16402 | a_dummy > > 16403 | mysub > > (2 rows) > > > > SELECT * FROM pg_replication_origin; > > roident | roname > > ---------+---------- > > 1 | pg_16402 > > 2 | pg_16403 > > (2 rows) > > > > The replication origin 1 is now used by a_dummy subscription. > > > > 6. Update the tuple on the publisher > > -- on publisher > > UPDATE test SET c = 'change after upgrade' WHERE id = 1; > > > > Then I got the following LOG: > > > > LOG: conflict detected on relation "public.test": > > conflict=update_origin_differs > > 2026-03-16 13:53:41.992 PDT [1247879] DETAIL: Updating the row that > > was modified by a different origin "pg_16402" in transaction 760 at > > 2026-03-16 13:53:33.448002-07: local row (1, replicated change), > > remote row (1, change after upgrade), replica identity (id)=(1). > > 2026-03-16 13:53:41.992 PDT [1247879] CONTEXT: processing remote data > > for replication origin "pg_16403" during message type "UPDATE" for > > replication target relation "public.test" in transaction 664, finished > > at 0/017ED910 > > > > I think that this patch works fine only when users don't use the > > replication origins. > > > > But is there any value of this patch without replication origins? I > thought the value of this patch is to detect conflicts correctly after > upgrade. IIUC, in some cases, the commit time on subscriber could > still be later than the conflicting transaction from publisher in > which case timing data can also be used for last_write_win kind of > strategy but without origin that also won't be possible. I don't have the specific example, but thought there might be use cases without logical replication, for example, pulling all the new records inserted within the last 30min to another system etc. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
-
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2026-03-18T14:42:59Z
<div>+ amit.kapila16@</div><div> </div><div>Hi,</div><div> </div><div><div>I will check for subscriptions and if there are any, there will be no migration of the pg_commit_timestamp directory (the behavior will be standard).</div><div> </div><div><blockquote><div>I think that this patch works fine only when users don't use the<br />replication origins. On the other hand, I think we should ensure that<br />subscriptions use the same replication origins even after upgrade in<br />cases where users are upgrading subscribers.</div></blockquote></div><div><div>I do not know how to do this.</div></div><div> </div><div><blockquote><div>Also, regarding the patch, with the following change we disallow the<br />case where users want to disable the commit timestamp in the new<br />cluster. IIUC migrating the commit timestamps is not mandatory in<br />practice, so I think we do the migration when applicable or raise a<br />warning if only the new cluster disables the commit timestamps.</div></blockquote></div><div> </div><div><div>This is a safer strategy to help you avoid losing the data that the user needs. I received a recommendation to do so.</div></div><div><div>Should I issue a warning about loss of transaction commit times?</div><div>Write the text I need to output.</div></div><div> </div></div><div>----------------</div><div>Кому: ls7777 (ls7777@yandex.ru);</div><div>Копия: Hayato Kuroda (Fujitsu) (kuroda.hayato@fujitsu.com), pgsql-hackers@postgresql.org, orlovmg@gmail.com;</div><div>Тема: Patch for migration of the pg_commit_ts directory;</div><div>17.03.2026, 02:04, "Masahiko Sawada" <sawada.mshk@gmail.com>:</div><blockquote><p>Hi,<br /><br />On Wed, Mar 11, 2026 at 10:51 AM ls7777 <<a href="mailto:ls7777@yandex.ru" rel="noopener noreferrer">ls7777@yandex.ru</a>> wrote:</p><blockquote><br /> Hello,<br /> I saw a big comment that I had missed.<br /> Added fixes in two patches:<br /> v11-0001-Migration-of-the-pg_commit_ts-directory.patch - main changes<br /> v11-0002-Migration-of-the-pg_commit_ts-directory-desc.patch - changes to documentation and comments<br /><br /><br /> ----------------<br /> Кому: 'ls7777' (<a href="mailto:ls7777@yandex.ru" rel="noopener noreferrer">ls7777@yandex.ru</a>);<br /> Копия: <a href="mailto:pgsql-hackers@postgresql.org" rel="noopener noreferrer">pgsql-hackers@postgresql.org</a>, <a href="mailto:orlovmg@gmail.com" rel="noopener noreferrer">orlovmg@gmail.com</a>;<br /> Тема: Patch for migration of the pg_commit_ts directory;<br /> 11.03.2026, 12:55, "Hayato Kuroda (Fujitsu)" <<a href="mailto:kuroda.hayato@fujitsu.com" rel="noopener noreferrer">kuroda.hayato@fujitsu.com</a>>:<br /><br /> Dear Sergey,<br /><br /><br /> I don't understand what I need to do.<br /> Subject: [PATCH v10] Migration of the pg_commit_ts directory<br /> I have specified a new version in the patch. Do I need to add anything else?<br /><br /><br /> When we see the commit message in postgres repo, you can see that each commit<br /> contain the background, what we did in the commit, considerations, and etc.<br /> Compared with them, the description you added seems insufficient.<br /> Of course the committer will take care, but Amit suggested [1] to write a draft.<br /> The basic style guide is determined in [2].<br /><br /> Actually I wrote a draft for the patch set in v9-0002, but you miss. You may<br /> able to refer it, and re-create new version or use the same one.<br /> </blockquote><p><br />I've reviewed this patch and have one question.<br /><br />The commit timestamp has not only the timestamp data but also<br />replication origin id. The replication origin id assigned to the<br />subscription on the new cluster is not necessarily the same as the<br />ones assigned on the old cluster. Therefore, if there is mismatch,<br />replication conflict 'update_origin_differs' could wrongly happen,<br />even though the new subscriber starts logical replication from the<br />same publisher. I've confirmed this behavior in the following<br />scenario:<br /><br />1. Setup publication.<br />-- on publisher<br />CREATE TABLE test (id int primary key, t text);<br />CREATE PUBLICATION pub FOR TABLE test;<br /><br />2. Setup subscription.<br />-- on subscriber<br />CREATE TABLE test (id int primary key, t text);<br />CREATE SUBSCRIPTION mysub CONNECTION '...' PUBLICATION pub;<br />CREATE SUBSCRIPTION a_dummy CONNECTION '...' PUBLICATION pub WITH<br />(connect = false, enabled = false);<br /><br />-- confirm that mysub's origin id is 1 and a_dummy's origin id is 2.<br />SELECT oid, subname FROM pg_subscription;<br /> oid | subname<br />-------+---------<br /> 16392 | mysub<br /> 16393 | a_dummy<br />(2 rows)<br /><br />SELECT * FROM pg_replication_origin;<br /> roident | roname<br />---------+----------<br /> 1 | pg_16392<br /> 2 | pg_16393<br />(2 rows)<br /><br />3. Insert a tuple to the test table and wait for it to be replicated.<br />-- on publisher<br />INSERT INTO test VALUES (1, 'replicated change');<br /><br />The change is applied on the subscription with replication origin id 1:<br />-- on subscriber<br />SELECT * FROM test, pg_xact_commit_timestamp_origin(test.xmin);<br /> id | c | timestamp | roident<br />----+-------------------+-------------------------------+---------<br /> 1 | replicated change | 2026-03-16 13:49:35.211316-07 | 1<br />(1 row)<br /><br />4. Do pg_upgrade<br />5. Check the replication origins in the new subscriber<br /><br />SELECT oid, subname FROM pg_subscription;<br /> oid | subname<br />-------+---------<br /> 16402 | a_dummy<br /> 16403 | mysub<br />(2 rows)<br /><br />SELECT * FROM pg_replication_origin;<br /> roident | roname<br />---------+----------<br /> 1 | pg_16402<br /> 2 | pg_16403<br />(2 rows)<br /><br />The replication origin 1 is now used by a_dummy subscription.<br /><br />6. Update the tuple on the publisher<br />-- on publisher<br />UPDATE test SET c = 'change after upgrade' WHERE id = 1;<br /><br />Then I got the following LOG:<br /><br />LOG: conflict detected on relation "public.test":<br />conflict=update_origin_differs<br />2026-03-16 13:53:41.992 PDT [1247879] DETAIL: Updating the row that<br />was modified by a different origin "pg_16402" in transaction 760 at<br />2026-03-16 13:53:33.448002-07: local row (1, replicated change),<br />remote row (1, change after upgrade), replica identity (id)=(1).<br />2026-03-16 13:53:41.992 PDT [1247879] CONTEXT: processing remote data<br />for replication origin "pg_16403" during message type "UPDATE" for<br />replication target relation "public.test" in transaction 664, finished<br />at 0/017ED910<br /><br />I think that this patch works fine only when users don't use the<br />replication origins. On the other hand, I think we should ensure that<br />subscriptions use the same replication origins even after upgrade in<br />cases where users are upgrading subscribers.<br /><br />Also, regarding the patch, with the following change we disallow the<br />case where users want to disable the commit timestamp in the new<br />cluster. IIUC migrating the commit timestamps is not mandatory in<br />practice, so I think we do the migration when applicable or raise a<br />warning if only the new cluster disables the commit timestamps.<br /><br />+ if (!commit_ts_is_enabled &&<br />+ old_cluster.controldata.chkpnt_newstCommitTsxid > 0)<br />+ pg_fatal("\"track_commit_timestamp\" must be \"on\" but is set<br />to \"off\"");<br />+<br /><br />Regards,<br /> </p>--<br />Masahiko Sawada<br />Amazon Web Services: <a href="https://aws.amazon.com/" rel="noopener noreferrer">https://aws.amazon.com</a></blockquote> -
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-03-19T05:30:41Z
Dear Sergey, > This is a safer strategy to help you avoid losing the data that the user needs. I received a recommendation to do so. I think we need stronger use-case to proceed like that. Did you get recommendations from customers/users? If so, why do they want to avoid losing the commit timestamp? Are they collaborating some other systems? Your voice/experience is really needed. Best regards, Hayato Kuroda FUJITSU LIMITED
-
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2026-03-19T07:02:15Z
<div><div>Hi,</div><div> </div><div>We use the transaction commit time to answer questions about when a record in any table was changed.</div><div>The main use is to detect application errors. It is used only by DB administrators. Transaction information is stored for up to 2 years.</div><div>The "temporary" nature of this information prevents the transaction commit time from being used more widely.</div></div><div> </div><div><div>We copied the pg_commit_ts directory manually. But it's unreliable.</div><div>Can I add a command line option?</div><div>So that the decision to migrate pg_commit_ts is made by the user ?</div></div><div> </div><div>----------------</div><div>Кому: 'ls7777' (ls7777@yandex.ru), Masahiko Sawada (sawada.mshk@gmail.com);</div><div>Копия: pgsql-hackers@postgresql.org, orlovmg@gmail.com, amit.kapila16@gmail.com;</div><div>Тема: Patch for migration of the pg_commit_ts directory;</div><div>19.03.2026, 10:31, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>:</div><blockquote><p>Dear Sergey,<br /> </p><blockquote> This is a safer strategy to help you avoid losing the data that the user needs. I received a recommendation to do so.</blockquote><p><br />I think we need stronger use-case to proceed like that.<br />Did you get recommendations from customers/users? If so, why do they want to<br />avoid losing the commit timestamp? Are they collaborating some other systems?<br />Your voice/experience is really needed.<br /><br />Best regards,<br />Hayato Kuroda<br />FUJITSU LIMITED<br /> </p></blockquote>
-
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2026-03-19T10:50:59Z
<div>Hi,</div><div> </div><div><div>As it turned out, when migrating to the "roident" field of the pg_replication_origin table, "random" values are obtained.</div><div>Update can restore the old values. For example:</div><div>UPDATE pg_replication_origin r SET roident=1 from pg_subscription s WHERE r.roname='pg_' || s.oid and s.subname='subcribtion1';</div></div><div> </div><div><div>I did not find any such code in the pg_upgrade source code. I have no idea how to do this. It is possible to describe in the comments the solution to the problem with {UPDATE|DELETE}_ORIGIN_DIFFERS conflicts.</div><div> </div></div><div>----------------</div><div>Кому: ls7777 (ls7777@yandex.ru);</div><div>Копия: Hayato Kuroda (Fujitsu) (kuroda.hayato@fujitsu.com), pgsql-hackers@postgresql.org, orlovmg@gmail.com;</div><div>Тема: Patch for migration of the pg_commit_ts directory;</div><div>17.03.2026, 02:04, "Masahiko Sawada" <sawada.mshk@gmail.com>:</div><blockquote><p>Hi,<br /><br />On Wed, Mar 11, 2026 at 10:51 AM ls7777 <<a href="mailto:ls7777@yandex.ru" rel="noopener noreferrer">ls7777@yandex.ru</a>> wrote:</p><blockquote><br /> Hello,<br /> I saw a big comment that I had missed.<br /> Added fixes in two patches:<br /> v11-0001-Migration-of-the-pg_commit_ts-directory.patch - main changes<br /> v11-0002-Migration-of-the-pg_commit_ts-directory-desc.patch - changes to documentation and comments<br /><br /><br /> ----------------<br /> Кому: 'ls7777' (<a href="mailto:ls7777@yandex.ru" rel="noopener noreferrer">ls7777@yandex.ru</a>);<br /> Копия: <a href="mailto:pgsql-hackers@postgresql.org" rel="noopener noreferrer">pgsql-hackers@postgresql.org</a>, <a href="mailto:orlovmg@gmail.com" rel="noopener noreferrer">orlovmg@gmail.com</a>;<br /> Тема: Patch for migration of the pg_commit_ts directory;<br /> 11.03.2026, 12:55, "Hayato Kuroda (Fujitsu)" <<a href="mailto:kuroda.hayato@fujitsu.com" rel="noopener noreferrer">kuroda.hayato@fujitsu.com</a>>:<br /><br /> Dear Sergey,<br /><br /><br /> I don't understand what I need to do.<br /> Subject: [PATCH v10] Migration of the pg_commit_ts directory<br /> I have specified a new version in the patch. Do I need to add anything else?<br /><br /><br /> When we see the commit message in postgres repo, you can see that each commit<br /> contain the background, what we did in the commit, considerations, and etc.<br /> Compared with them, the description you added seems insufficient.<br /> Of course the committer will take care, but Amit suggested [1] to write a draft.<br /> The basic style guide is determined in [2].<br /><br /> Actually I wrote a draft for the patch set in v9-0002, but you miss. You may<br /> able to refer it, and re-create new version or use the same one.<br /> </blockquote><p><br />I've reviewed this patch and have one question.<br /><br />The commit timestamp has not only the timestamp data but also<br />replication origin id. The replication origin id assigned to the<br />subscription on the new cluster is not necessarily the same as the<br />ones assigned on the old cluster. Therefore, if there is mismatch,<br />replication conflict 'update_origin_differs' could wrongly happen,<br />even though the new subscriber starts logical replication from the<br />same publisher. I've confirmed this behavior in the following<br />scenario:<br /><br />1. Setup publication.<br />-- on publisher<br />CREATE TABLE test (id int primary key, t text);<br />CREATE PUBLICATION pub FOR TABLE test;<br /><br />2. Setup subscription.<br />-- on subscriber<br />CREATE TABLE test (id int primary key, t text);<br />CREATE SUBSCRIPTION mysub CONNECTION '...' PUBLICATION pub;<br />CREATE SUBSCRIPTION a_dummy CONNECTION '...' PUBLICATION pub WITH<br />(connect = false, enabled = false);<br /><br />-- confirm that mysub's origin id is 1 and a_dummy's origin id is 2.<br />SELECT oid, subname FROM pg_subscription;<br /> oid | subname<br />-------+---------<br /> 16392 | mysub<br /> 16393 | a_dummy<br />(2 rows)<br /><br />SELECT * FROM pg_replication_origin;<br /> roident | roname<br />---------+----------<br /> 1 | pg_16392<br /> 2 | pg_16393<br />(2 rows)<br /><br />3. Insert a tuple to the test table and wait for it to be replicated.<br />-- on publisher<br />INSERT INTO test VALUES (1, 'replicated change');<br /><br />The change is applied on the subscription with replication origin id 1:<br />-- on subscriber<br />SELECT * FROM test, pg_xact_commit_timestamp_origin(test.xmin);<br /> id | c | timestamp | roident<br />----+-------------------+-------------------------------+---------<br /> 1 | replicated change | 2026-03-16 13:49:35.211316-07 | 1<br />(1 row)<br /><br />4. Do pg_upgrade<br />5. Check the replication origins in the new subscriber<br /><br />SELECT oid, subname FROM pg_subscription;<br /> oid | subname<br />-------+---------<br /> 16402 | a_dummy<br /> 16403 | mysub<br />(2 rows)<br /><br />SELECT * FROM pg_replication_origin;<br /> roident | roname<br />---------+----------<br /> 1 | pg_16402<br /> 2 | pg_16403<br />(2 rows)<br /><br />The replication origin 1 is now used by a_dummy subscription.<br /><br />6. Update the tuple on the publisher<br />-- on publisher<br />UPDATE test SET c = 'change after upgrade' WHERE id = 1;<br /><br />Then I got the following LOG:<br /><br />LOG: conflict detected on relation "public.test":<br />conflict=update_origin_differs<br />2026-03-16 13:53:41.992 PDT [1247879] DETAIL: Updating the row that<br />was modified by a different origin "pg_16402" in transaction 760 at<br />2026-03-16 13:53:33.448002-07: local row (1, replicated change),<br />remote row (1, change after upgrade), replica identity (id)=(1).<br />2026-03-16 13:53:41.992 PDT [1247879] CONTEXT: processing remote data<br />for replication origin "pg_16403" during message type "UPDATE" for<br />replication target relation "public.test" in transaction 664, finished<br />at 0/017ED910<br /><br />I think that this patch works fine only when users don't use the<br />replication origins. On the other hand, I think we should ensure that<br />subscriptions use the same replication origins even after upgrade in<br />cases where users are upgrading subscribers.<br /><br />Also, regarding the patch, with the following change we disallow the<br />case where users want to disable the commit timestamp in the new<br />cluster. IIUC migrating the commit timestamps is not mandatory in<br />practice, so I think we do the migration when applicable or raise a<br />warning if only the new cluster disables the commit timestamps.<br /><br />+ if (!commit_ts_is_enabled &&<br />+ old_cluster.controldata.chkpnt_newstCommitTsxid > 0)<br />+ pg_fatal("\"track_commit_timestamp\" must be \"on\" but is set<br />to \"off\"");<br />+<br /><br />Regards,<br /> </p>--<br />Masahiko Sawada<br />Amazon Web Services: <a href="https://aws.amazon.com/" rel="noopener noreferrer">https://aws.amazon.com</a></blockquote> -
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2026-03-21T08:59:11Z
Hi, The patch has been significantly updated. 1. Added the --pg-commit-ts command line key for pg_upgrade. Migration of the pg_commit_ts directory will be performed only if the user explicitly wishes. 2. The roident values are being transferred from pg_replication_origin to a new cluster to resolve conflict messages {UPDATE|DELETE}_ORIGIN_DIFFERS. During testing, it turned out that the replication sources created by the pg_replication_origin_create('no_link_sub_4') functions are not migrated to the new cluster. This is probably a bug. The principle of operation: A query to the old cluster generates SQL commands for roident adjustments, which are then executed in the new cluster. -
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-03-27T02:12:23Z
Dear Sergey, Sorry for being late, I was busy in another project. Not sure we have enough time to rush this for PG19 because some new parts are added recently. Personally considered, the patch might be able to separate into two parts. 0001 contains changes to migrate the replication origin, and 0002 contains to migrate the commit_ts directory. For 0001, we may need a new SQL function to migrate replication origins, and it could be used only while in the binary upgrade mode like binary_upgrade_add_sub_rel_state(). We must not directly update the system catalog via UPDATE command. 0002 might be the same as what we discussed earlier. > 1. Added the --pg-commit-ts command line key for pg_upgrade. > Migration of the pg_commit_ts directory will be performed only if the user explicitly wishes. I cannot find the reason why you changed here. Can you share me your opinion? Best regards, Hayato Kuroda FUJITSU LIMITED
-
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2026-03-27T15:16:25Z
<div>Hi,</div><div> </div><div><div>Why do I need the --pg-commit-ts key?</div><div>1. Users who do not need to migrate the pg_commit_ts directory (regardless of the value of the pg_commit_timestamp parameter) will receive the usual pg_upgrade behavior.</div><div>2. Users who need to move pg_commit_ts directory may already have their own scripts for copying pg_commit_ts directory, and decide to continue using them. There will also be no surprises for them when using pg_upgrage.</div><div>3. pg_commit_ts migration turned out to be more complicated than I expected. And the pg_replication_origin migration was affected. I did the migration the way I can (update tables), but it's architecturally incorrect. Therefore, using the command line key can be used as a temporary solution that will be removed after creating an architecturally correct solution.</div></div><div> </div><div><div>If the current solution with update pg_replication_origin is not suitable. I will remove these updates and will not migrate pg_commit_ts if there is a non-empty pg_replication_origin in the old cluster. After migration, the user should not receive conflict messages. This may give the impression of an erroneous migration.</div><div>I would like to keep the command line key.</div></div><div> </div><div>----------------</div><div>Кому: 'ls7777' (ls7777@yandex.ru), Masahiko Sawada (sawada.mshk@gmail.com);</div><div>Копия: pgsql-hackers@postgresql.org, orlovmg@gmail.com, amit.kapila16@gmail.com;</div><div>Тема: Patch for migration of the pg_commit_ts directory;</div><div>27.03.2026, 07:12, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>:</div><blockquote><p>Dear Sergey,<br /><br />Sorry for being late, I was busy in another project.<br />Not sure we have enough time to rush this for PG19 because some new parts are<br />added recently.<br /><br />Personally considered, the patch might be able to separate into two parts.<br />0001 contains changes to migrate the replication origin, and 0002 contains to<br />migrate the commit_ts directory. For 0001, we may need a new SQL function to<br />migrate replication origins, and it could be used only while in the binary upgrade<br />mode like binary_upgrade_add_sub_rel_state(). We must not directly update the<br />system catalog via UPDATE command. 0002 might be the same as what we discussed<br />earlier.<br /> </p><blockquote> 1. Added the --pg-commit-ts command line key for pg_upgrade.<br /> Migration of the pg_commit_ts directory will be performed only if the user explicitly wishes.</blockquote><p><br />I cannot find the reason why you changed here. Can you share me your opinion?<br /><br /><br />Best regards,<br />Hayato Kuroda<br />FUJITSU LIMITED<br /> </p></blockquote>
-
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2026-04-05T18:27:00Z
Hi, I fixed the compiler warnings. I want to ask. Do I need to restore replication sources in my patch created by calling the pg_replication_origin_create function? Currently, these replication sources are not being migrated to the new cluster. Maybe it should be done later. If there are users who use it. ---------------- Кому: Hayato Kuroda (Fujitsu) (kuroda.hayato@fujitsu.com), Masahiko Sawada (sawada.mshk@gmail.com); Копия: pgsql-hackers@postgresql.org, orlovmg@gmail.com, amit.kapila16@gmail.com; Тема: Patch for migration of the pg_commit_ts directory; 21.03.2026, 13:59, "ls7777" <ls7777@yandex.ru>: Hi, The patch has been significantly updated. 1. Added the --pg-commit-ts command line key for pg_upgrade. Migration of the pg_commit_ts directory will be performed only if the user explicitly wishes. 2. The roident values are being transferred from pg_replication_origin to a new cluster to resolve conflict messages {UPDATE|DELETE}_ORIGIN_DIFFERS. During testing, it turned out that the replication sources created by the pg_replication_origin_create('no_link_sub_4') functions are not migrated to the new cluster. This is probably a bug. The principle of operation: A query to the old cluster generates SQL commands for roident adjustments, which are then executed in the new cluster. -
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2026-04-06T04:34:08Z
Hi, fix error compile ---------------- Кому: Hayato Kuroda (Fujitsu) (kuroda.hayato@fujitsu.com), Masahiko Sawada (sawada.mshk@gmail.com); Копия: pgsql-hackers@postgresql.org, orlovmg@gmail.com, amit.kapila16@gmail.com; Тема: Patch for migration of the pg_commit_ts directory; 05.04.2026, 23:27, "ls7777" <ls7777@yandex.ru>: Hi, I fixed the compiler warnings. I want to ask. Do I need to restore replication sources in my patch created by calling the pg_replication_origin_create function? Currently, these replication sources are not being migrated to the new cluster. Maybe it should be done later. If there are users who use it. ---------------- Кому: Hayato Kuroda (Fujitsu) (kuroda.hayato@fujitsu.com), Masahiko Sawada (sawada.mshk@gmail.com); Копия: pgsql-hackers@postgresql.org, orlovmg@gmail.com, amit.kapila16@gmail.com; Тема: Patch for migration of the pg_commit_ts directory; 21.03.2026, 13:59, "ls7777" <ls7777@yandex.ru>: Hi, The patch has been significantly updated. 1. Added the --pg-commit-ts command line key for pg_upgrade. Migration of the pg_commit_ts directory will be performed only if the user explicitly wishes. 2. The roident values are being transferred from pg_replication_origin to a new cluster to resolve conflict messages {UPDATE|DELETE}_ORIGIN_DIFFERS. During testing, it turned out that the replication sources created by the pg_replication_origin_create('no_link_sub_4') functions are not migrated to the new cluster. This is probably a bug. The principle of operation: A query to the old cluster generates SQL commands for roident adjustments, which are then executed in the new cluster. -
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-05-12T00:42:28Z
Dear Sergey, I found that Ajin started another thread which migrates replication origins [1]. So how about purging the part from this patch? I.e., commit_ts migrations would be migrated well if both patches are pushed. To keep the behavior consistent, [1] may have to be pushed first. Personally considered, added checks in the patch may look bit ad-hoc, and we should fix from the root cause. [1]: https://commitfest.postgresql.org/patch/6743/
-
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-05-27T05:24:54Z
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
-
Re: Patch for migration of the pg_commit_ts directory
ls7777 <ls7777@yandex.ru> — 2026-05-29T16:08:56Z
<div>Hi,</div><div> </div><div><div>During copying, the copy_subdir_files function cleans up the target directory with the remove_new_subdir(new_subdir, true) function.</div><div> </div></div><div><br /></div><div><br /></div><div>----------------</div> <div>Кому: 'ls7777' (ls7777@yandex.ru);<br /></div> <div>Копия: pgsql-hackers@postgresql.org, orlovmg@gmail.com, amit.kapila16@gmail.com, Masahiko Sawada (sawada.mshk@gmail.com);<br /></div> <div>Тема: Patch for migration of the pg_commit_ts directory;<br /></div> <div>27.05.2026, 10:25, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>:<br /></div> <blockquote><p>Dear Sergey,<br /><br />While considering the feature once again, I came up with another issue.<br /><br />When pg_upgrade migrates objects to a new instance, it normally checks whether<br />the new instance does not have existing objects. E.g., check_new_cluster_is_empty()<br />checks if no tables exist, and check_new_cluster_replication_slots() ensures<br />there are no logical slots.<br /><br />Based on that, how should we handle the commit timestamp? Since the patch simply<br />copies the pg_commit_ts subdir, existing entries in the new cluster will be overwritten.<br />So we may have to check if the commit_ts entries are empty when the pg_upgrade<br />command runs, but not sure how to do that.<br /><br />Best regards,<br />Hayato Kuroda<br />FUJITSU LIMITED<br /> <br /></p></blockquote> -
RE: Patch for migration of the pg_commit_ts directory
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-06-01T01:25:31Z
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