From: Andy Fan <zhihuifan1213@163.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: A assert failure when initdb with track_commit_timestamp=on
Date: Wed, 02 Jul 2025 01:38:18 +0000
Message-ID: <87ikkbmkxh.fsf@163.com> (raw)
In-Reply-To: <aGSBcvlXY9FDkcsr@paquier.xyz>
References: <87plejmnpy.fsf@163.com>
<aGSBcvlXY9FDkcsr@paquier.xyz>
Michael Paquier <michael@paquier.xyz> writes:
Hi,
> On Wed, Jul 02, 2025 at 12:38:01AM +0000, Andy Fan wrote:
>> However this is not true in BootstrapMode, this failure is masked by
>> default because TransactionTreeSetCommitTsData returns fast when
>> track_commit_timestamp is off.
>
> Agreed that there is no point in registering a commit timestamp in
> the cases of a frozen and bootstrap XIDs. I would recommend to keep
> the assertion in TransactionIdSetCommitTs(), though, that still looks
> useful to me for the many callers of this routine, at least as a
> sanity check.
Yes, The assert also guard the InvalidTransactionId. So I removed this
solution in v2. Another reason for this is: if we allowed
BooststrapTransactionId in the commit_ts, it introduces something new to
this module when initdb with track_commit_timestamp=on. This risk might
be very low, but it can be avoided easily with the another solution.
>
> I did not check, but usually we apply filters based on
> IsBootstrapProcessingMode() for code paths that we do not want to
> reach while in bootstrap mode. Could the same be done here?
I think you are right. so I used IsBootstrapProcessingMode in v2.
--
Best Regards
Andy Fan
Attachments:
[text/x-diff] v2-0001-Don-t-record-commit_ts-in-bootstarp-mode.patch (1.2K, ../87ikkbmkxh.fsf@163.com/2-v2-0001-Don-t-record-commit_ts-in-bootstarp-mode.patch)
download | inline diff:
From def54f2c6a73aec74aebde9b687612f65a8bcdd5 Mon Sep 17 00:00:00 2001
From: Andy Fan <zhihuifan1213@163.com>
Date: Wed, 2 Jul 2025 01:23:34 +0000
Subject: [PATCH v2 1/1] Don't record commit_ts in bootstarp mode.
This case would raise Assert failure in TransactionIdSetCommitTs when
initdb with track_commit_timestamp=on.
Another option might be allowing BooststrapTransactionId in
TransactionIdSetCommitTs, but this adds a new change in
commit_ts module, which risk could be avoided easily with current
solution.
---
src/backend/access/transam/commit_ts.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.cindex 113fae1437a..1aa2dc450f5 100644--- a/src/backend/access/transam/commit_ts.c+++ b/src/backend/access/transam/commit_ts.c@@ -157,6 +157,12 @@ TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
if (!commitTsShared->commitTsActive)
return;
+ /*+ * Don't bother to record commit_ts for Booststrap mode.+ */+ if (IsBootstrapProcessingMode())+ return;+
/*
* Figure out the latest Xid in this batch: either the last subxid if
* there's any, otherwise the parent xid.
--
2.45.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: zhihuifan1213@163.com, michael@paquier.xyz
Subject: Re: A assert failure when initdb with track_commit_timestamp=on
In-Reply-To: <87ikkbmkxh.fsf@163.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox