"Hayato Kuroda (Fujitsu)" <kuroda.hay...@fujitsu.com> writes:
> Dear Michael, Fujii-san, > >> Ah, indeed, so it was reported a couple of months ago. I am not sure >> that the argument about all the other GUCs potentially impacted holds >> much value; we are talking about a specific code path. > > Yeah, I did report but sadly it was missed by others :-(. To clarify, > The current patch looks good to me. Then I'd thank Michael to watch the maillist closely this time. I checked the fix suggested by Hayato, I think his patch is better than me because his patch checks at the startup time while my patch checks at each time of RecordTransactionCommit. So v3 takes his patch. v3 also added the testcase suggested by Michael for test coverage, it clearly proves the bug is fixed now. -- Best Regards Andy Fan
>From dab43830af019896ef1811aeeeb5d1e4e2bcccea Mon Sep 17 00:00:00 2001 From: Andy Fan <zhihuifan1...@163.com> Date: Thu, 3 Jul 2025 13:04:02 +0000 Subject: [PATCH v3 1/1] Avoid activating commit_ts while bootstrap. TransactionIdSetCommitTs had an Assert failure when initdb with track_commit_timestamp=on before this commit because it thought all the provided xids are normal xid. So avoiding the activating commit_ts in bootstrap mode could fix this issue. Test case is also enhanced to discover this issue. Author: Hayato Kuroda <kuroda.hay...@fujitsu.com> Author: Andy Fan <zhihuifan1...@163.com> Reviewed-by: Michael Paquier <mich...@paquier.xyz> Reviewed-by: Bertrand Drouvot <bertranddrouvot...@gmail.com> Reviewed-by: Fujii Masao <masao.fu...@oss.nttdata.com> Discussion: https://postgr.es/m/oscpr01mb14966ff9e4c4145f37b937e52f5...@oscpr01mb14966.jpnprd01.prod.outlook.com Discussion: https://postgr.es/m/87plejmnpy.fsf%40163.com --- src/backend/access/transam/commit_ts.c | 7 +++++++ src/test/modules/commit_ts/t/001_base.pl | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c index 113fae1437a..15078698cf9 100644 --- a/src/backend/access/transam/commit_ts.c +++ b/src/backend/access/transam/commit_ts.c @@ -707,6 +707,13 @@ ActivateCommitTs(void) TransactionId xid; int64 pageno; + /* + * commit_ts assumes that we are not in the bootstrap mode: skip the + * activation. + */ + if (IsBootstrapProcessingMode()) + return; + /* If we've done this already, there's nothing to do */ LWLockAcquire(CommitTsLock, LW_EXCLUSIVE); if (commitTsShared->commitTsActive) diff --git a/src/test/modules/commit_ts/t/001_base.pl b/src/test/modules/commit_ts/t/001_base.pl index 1953b18f6b3..91983fc5179 100644 --- a/src/test/modules/commit_ts/t/001_base.pl +++ b/src/test/modules/commit_ts/t/001_base.pl @@ -11,7 +11,7 @@ use Test::More; use PostgreSQL::Test::Cluster; my $node = PostgreSQL::Test::Cluster->new('foxtrot'); -$node->init; +$node->init(extra => ['-c', "track_commit_timestamp=on"]); $node->append_conf('postgresql.conf', 'track_commit_timestamp = on'); $node->start; -- 2.45.1
