From: Michael Paquier <michael@paquier.xyz>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Andy Fan <zhihuifan1213@163.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: A assert failure when initdb with track_commit_timestamp=on
Date: Thu, 3 Jul 2025 09:28:25 +0900
Message-ID: <aGXOqUsnosMSWevC@paquier.xyz> (raw)
In-Reply-To: <aGT153TXgOW/nPtQ@ip-10-97-1-34.eu-west-3.compute.internal>
References: <87plejmnpy.fsf@163.com>
<aGSBcvlXY9FDkcsr@paquier.xyz>
<87ikkbmkxh.fsf@163.com>
<aGT153TXgOW/nPtQ@ip-10-97-1-34.eu-west-3.compute.internal>
On Wed, Jul 02, 2025 at 09:03:35AM +0000, Bertrand Drouvot wrote:
> + * Don't bother to record commit_ts for Booststrap mode.
>
> typo: s/Booststrap/Bootstrap/
>
> Also, grep on "Bootstrap mode" and "bootstrap mode" gives much more occurrences
> for the later, so maybe use "bootstrap mode" instead?
>
> === 2
>
> The FrozenTransactionId case is missing in v2, is that on purpose?
There may be an argument about bypassing the frozen case as well, but
I cannot get excited about that because it can also be very useful to
detect problems as calling this routine with a frozen XID would be
really wrong. The assert in TransactionIdSetCommitTs() is enough for
this purpose, no need to add something else. As of now
TransactionTreeSetCommitTsData() is only called with an active XID at
commit, but if somebody has too much imagination in a module..
As a whole I think that the addition of IsBootstrapProcessingMode() in
TransactionTreeSetCommitTsData() is enough. There may be an argument
about backpatching this change. However, nobody has complained about
the idea of enabling the commit_ts module while bootstrapping, so I'd
keep it as a HEAD-only thing and be conservative.
A second thing I would suggest is some test coverage, with the
addition of extra => [ '-c', "track_commit_timestamp=on=on" ] in an
init() method called in one of the script tests of
src/test/modules/commit_ts/t. Just changing 001_base.pl should be
enough to trigger your original failure, making sure that this never
breaks again.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
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: michael@paquier.xyz, bertranddrouvot.pg@gmail.com, zhihuifan1213@163.com
Subject: Re: A assert failure when initdb with track_commit_timestamp=on
In-Reply-To: <aGXOqUsnosMSWevC@paquier.xyz>
* 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