Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix recovery of 2PC transaction during crash recovery

  1. The same 2PC data maybe recovered twice

    suyu.cmj <mengjuan.cmj@alibaba-inc.com> — 2023-07-06T02:02:15Z

    Hi, all. I want to report a bug about recovery of 2pc data, in current implementation of crash recovery, there are two ways to recover 2pc data:
1、before redo, func restoreTwoPhaseData() will restore 2pc data those xid < ShmemVariableCache->nextXid, which is initialized from checkPoint.nextXid;
2、during redo, func xact_redo() will add 2pc from wal;
The following scenario may cause the same 2pc to be added repeatedly：
1、start creating checkpoint_1, checkpoint_1.redo is set as curInsert;
2、before set checkPoint_1.nextXid, a new 2pc is prepared, suppose the xid of this 2pc is 100, and then ShmemVariableCache->nextXid will be advanced as 101;
3、checkPoint_1.nextXid is set as 101;
4、in CheckPointTwoPhase() of checkpoint_1, 2pc_100 won't be copied to disk because its prepare_end_lsn > checkpoint_1.redo;
5、checkPoint_1 is finished, after checkpoint_timeout, start creating checkpoint_2;
6、during checkpoint_2, data of 2pc_100 will be copied to disk;
7、before UpdateControlFile() of checkpoint_2, crash happened;
8、during crash recovery, redo will start from checkpoint_1, and 2pc_100 will be restored first by restoreTwoPhaseData() because xid_100 < checkPoint_1.nextXid, which is 101; 
9、because prepare_start_lsn of 2pc_100 > checkpoint_1.redo, 2pc_100 will be added again by xact_redo() during wal replay, resulting in the same 2pc data being added twice;
10、In RecoverPreparedTransactions() -> lock_twophase_recover(), lock the same 2pc will cause panic.
Is the above scenario reasonable, and do you have any good ideas for fixing this bug?
Thanks & Best Regard

  2. 回复：The same 2PC data maybe recovered twice

    suyu.cmj <mengjuan.cmj@alibaba-inc.com> — 2023-07-07T09:48:39Z

    Hi, all
I add a patch for pg11 to fix this bug, hope you can check it.
Thanks & Best Regard
------------------------------------------------------------------
发件人：蔡梦娟(玊于) <mengjuan.cmj@alibaba-inc.com>
发送时间：2023年7月6日(星期四) 10:02
收件人：pgsql-hackers <pgsql-hackers@postgresql.org>
抄　送：pgsql-bugs <pgsql-bugs@postgresql.org>
主　题：The same 2PC data maybe recovered twice
Hi, all. I want to report a bug about recovery of 2pc data, in current implementation of crash recovery, there are two ways to recover 2pc data:
1、before redo, func restoreTwoPhaseData() will restore 2pc data those xid < ShmemVariableCache->nextXid, which is initialized from checkPoint.nextXid;
2、during redo, func xact_redo() will add 2pc from wal;
The following scenario may cause the same 2pc to be added repeatedly：
1、start creating checkpoint_1, checkpoint_1.redo is set as curInsert;
2、before set checkPoint_1.nextXid, a new 2pc is prepared, suppose the xid of this 2pc is 100, and then ShmemVariableCache->nextXid will be advanced as 101;
3、checkPoint_1.nextXid is set as 101;
4、in CheckPointTwoPhase() of checkpoint_1, 2pc_100 won't be copied to disk because its prepare_end_lsn > checkpoint_1.redo;
5、checkPoint_1 is finished, after checkpoint_timeout, start creating checkpoint_2;
6、during checkpoint_2, data of 2pc_100 will be copied to disk;
7、before UpdateControlFile() of checkpoint_2, crash happened;
8、during crash recovery, redo will start from checkpoint_1, and 2pc_100 will be restored first by restoreTwoPhaseData() because xid_100 < checkPoint_1.nextXid, which is 101; 
9、because prepare_start_lsn of 2pc_100 > checkpoint_1.redo, 2pc_100 will be added again by xact_redo() during wal replay, resulting in the same 2pc data being added twice;
10、In RecoverPreparedTransactions() -> lock_twophase_recover(), lock the same 2pc will cause panic.
Is the above scenario reasonable, and do you have any good ideas for fixing this bug?
Thanks & Best Regard

  3. Re: The same 2PC data maybe recovered twice

    Andy Fan <zhihui.fan1213@gmail.com> — 2023-07-12T02:57:44Z

    Hi:



On Sat, Jul 8, 2023 at 2:53 AM 蔡梦娟(玊于) <mengjuan.cmj@alibaba-inc.com> wrote:

> Hi, all
> I add a patch for pg11 to fix this bug, hope you can check it.
>
>
Thanks for the bug report and patch!  Usually we talk about bugs
against the master branch,  no people want to check out a history
branch and do the discussion there:)  This  bug is reproducible on
the master IIUC.

I dislike the patch here because it uses more CPU cycles to detect
duplication for every 2pc record.  How many CPU cycles we use
depends on how many 2pc are used.  How about detecting such
duplication only at restoreTwoPhaseData stage?  Instead of

ProcessTwoPhaseBuffer:
if (TransactionIdFollowsOrEquals(xid, origNextXid))
{
  ...
ereport(WARNING,
(errmsg("removing future two-phase state file for transaction %u",
xid)));
RemoveTwoPhaseFile(xid, true);
  ...
}

we use:

if (TwoPhaseFileHeader.startup_lsn > checkpoint.redo)
{
ereport(WARNING,
(errmsg("removing future two-phase state file for transaction %u",
xid)));
}

We have several advantages with this approach. a).  We only care
about the restoreTwoPhaseData, not for every WAL record recovery.
b).  We use constant comparison rather than an-array-for-loop. c).
It is better design since we avoid the issue at the first place rather
than allowing it at the first stage and fix that at the following stage.

The only blocker I know is currently we don't write startup_lsn into
the  2pc checkpoint file and if we do that, the decode on the old 2pc
file will fail.  We also have several choices here.

a). Notify users to complete all the pending 2pc before upgrading
within manual.  b). Use a different MAGIC NUMBER in the 2pc
checkpoint file to distinguish the 2 versions.  Basically I prefer
the method a).

Any suggestion is welcome.


>
> ------------------------------------------------------------------
> 发件人：蔡梦娟(玊于) <mengjuan.cmj@alibaba-inc.com>
> 发送时间：2023年7月6日(星期四) 10:02
> 收件人：pgsql-hackers <pgsql-hackers@postgresql.org>
> 抄 送：pgsql-bugs <pgsql-bugs@postgresql.org>
> 主 题：The same 2PC data maybe recovered twice
>
> Hi, all. I want to report a bug about recovery of 2pc data, in current
> implementation of crash recovery, there are two ways to recover 2pc data:
> 1、before redo, func restoreTwoPhaseData() will restore 2pc data those xid
> < ShmemVariableCache->nextXid, which is initialized from
> checkPoint.nextXid;
> 2、during redo, func xact_redo() will add 2pc from wal;
>
> The following scenario may cause the same 2pc to be added repeatedly：
> 1、start creating checkpoint_1, checkpoint_1.redo is set as curInsert;
> 2、before set checkPoint_1.nextXid, a new 2pc is prepared, suppose the xid
> of this 2pc is 100, and then ShmemVariableCache->nextXid will be advanced
> as 101;
> 3、checkPoint_1.nextXid is set as 101;
> 4、in CheckPointTwoPhase() of checkpoint_1, 2pc_100 won't be copied to
> disk because its prepare_end_lsn > checkpoint_1.redo;
> 5、checkPoint_1 is finished, after checkpoint_timeout, start creating
> checkpoint_2;
> 6、during checkpoint_2, data of 2pc_100 will be copied to disk;
> 7、before UpdateControlFile() of checkpoint_2, crash happened;
> 8、during crash recovery, redo will start from checkpoint_1, and 2pc_100
> will be restored first by restoreTwoPhaseData() because xid_100 < checkPoint_1.nextXid,
> which is 101;
> 9、because prepare_start_lsn of 2pc_100 > checkpoint_1.redo, 2pc_100 will
> be added again by xact_redo() during wal replay, resulting in the same
> 2pc data being added twice;
> 10、In RecoverPreparedTransactions() -> lock_twophase_recover(), lock the
> same 2pc will cause panic.
>
> Is the above scenario reasonable, and do you have any good ideas for
> fixing this bug?
>
> Thanks & Best Regard
>
>

-- 
Best Regards
Andy Fan

  4. Re: The same 2PC data maybe recovered twice

    suyu.cmj <mengjuan.cmj@alibaba-inc.com> — 2023-07-12T07:20:57Z

    Yes, this bug can also be reproduced on the master branch, and the corresponding reproduction patch is attached.
I also considered comparing the 2pc.prepare_start_lsn and checkpoint.redo in ProcessTwoPhaseBuffer before, but this method requires modifying the format of the 2pc checkpoint file, which will bring compatibility issues. Especially for released branches, assuming that a node has encountered this bug, it will not be able to start successfully due to FATAL during crash recovery, and therefore cannot manually commit previous two-phase transactions. Using magic number to distinguish 2pc checkpoint file versions can't solve the problem in the above scenario either.
For unreleased branches, writing 2pc.prepare_start_lsn into the checkpoint file will be a good solution, but for released branches, I personally think using WAL record to overwrite checkpoint data would be a more reasonable approach, What do you think?
Best Regards
suyu.cmj

  5. Re: The same 2PC data maybe recovered twice

    Michael Paquier <michael@paquier.xyz> — 2023-07-13T06:58:56Z

    On Wed, Jul 12, 2023 at 03:20:57PM +0800, suyu.cmj wrote:
> Yes, this bug can also be reproduced on the master branch, and the
> corresponding reproduction patch is attached.

That's an interesting reproducer with injection points.  It looks like
you've spent a lot of time investigating that.  So, basically, a
checkpoint fails after writing a 2PC file to disk, but before the redo
LSN has been updated.

> I also considered comparing the 2pc.prepare_start_lsn and
> checkpoint.redo in ProcessTwoPhaseBuffer before, but this method
> requires modifying the format of the 2pc checkpoint file, which will
> bring compatibility issues. Especially for released branches,
> assuming that a node has encountered this bug, it will not be able
> to start successfully due to FATAL during crash recovery, and
> therefore cannot manually commit previous two-phase
> transactions. Using magic number to distinguish 2pc checkpoint file
> versions can't solve the problem in the above scenario either. 
> For unreleased branches, writing 2pc.prepare_start_lsn into the
> checkpoint file will be a good solution, but for released branches,

Yes, changing anything in this format is a no-go.  Now, things could
be written so as the recovery code is able to handle multiple formats,
meaning that it would be able to feed from the a new format that
includes a LSN or something else for the comparison, but that would
not save from the case where 2PC files with the old format are still
around and a 2PC WAL record is replayed.

> I personally think using WAL record to overwrite checkpoint data
> would be a more reasonable approach, What do you think? 

The O(2) loop added in PrepareRedoAdd() to scan the set of 2PC
transactions stored in TwoPhaseState for the purpose of checking for a
duplicate sucks from a performance point of view, particularly for 
deployments with many 2PC transactions allowed.  It could delay
recovery a lot.  And actually, this is not completely correct, no?
It is OK to bypass the recovery of the same transaction if the server
has not reached a consistent state, but getting a duplicate when
consistency has been reached should lead to a hard failure.

One approach to avoid this O(2) would be to use a hash table to store
the 2PC entries, for example, rather than an array.  That would be
simple enough but such refactoring is rather scary from the point of
view of recovery.

And, actually, we could do something much more simpler than what's
been proposed on this thread..  PrepareRedoAdd() would be called when
scanning pg_twophase at the beginning of recovery, or when replaying a
PREPARE record, both aiming at adding an entry in shmem for the 2PC
transaction tracked.  Here is a simpler idea: why don't we just check
in PrepareRedoAdd() if the 2PC file of the transaction being recovery
is in pg_twophase/ when adding an entry from a WAL record?  If a
consistent point has *not* been reached by recovery and we find a file
on disk, then do nothing because we *know* thanks to
restoreTwoPhaseData() done at the beginning of recover that there is
an entry for this file.  If a consistent point has been reached in
recovery and we find a file on disk while replaying a WAL record for
the same 2PC file, then fail.  If there is no file in pg_twophase for
the record replayed, then add it to the array TwoPhaseState.

Adding a O(2) loop that checks for duplicates may be a good idea as a
cross-check if replaying a record, but I'd rather put that under an
USE_ASSERT_CHECKING so as there is no impact on production systems,
still we'd have some sanity checks for test setups.
--
Michael

  6. Re: The same 2PC data maybe recovered twice

    suyu.cmj <mengjuan.cmj@alibaba-inc.com> — 2023-07-17T06:26:56Z

    Yes, the method you proposed is simpler and more efficient. Following your idea, I have modified the corresponding patch, hope you can review it when you have time.
Best Regards
suyu.cmj

  7. Re: The same 2PC data maybe recovered twice

    Michael Paquier <michael@paquier.xyz> — 2023-07-17T07:59:53Z

    On Mon, Jul 17, 2023 at 02:26:56PM +0800, suyu.cmj wrote:
> Yes, the method you proposed is simpler and more
> efficient. Following your idea, I have modified the corresponding
> patch, hope you can review it when you have time.

I'll double-check that tomorrow, but yes, that's basically what I had
in mind.  Thanks for the patch!

+   char        path[MAXPGPATH];
+   struct stat stat_buf;
These two variables can be declared in the code block added by the
patch where start_lsn is valid.

+    ereport(FATAL,
+            (errmsg("found unexpected duplicate two-phase
transaction:%u in pg_twophase, check for data correctness.",
+                    hdr->xid)));

The last part of this sentence has no need to be IMO, because it is
misleading when building without assertions.  How about a single
FATAL/WARNING like that:
- errmsg: "could not recover two-phase state file for transaction %u"
- errdetail: "Two-phase state file has been found in WAL record %X/%X
but this transaction has already been restored from disk."

Then a WARNING simply means that we've skipped the record entirely.
--
Michael

  8. Re: The same 2PC data maybe recovered twice

    suyu.cmj <mengjuan.cmj@alibaba-inc.com> — 2023-07-17T09:20:00Z

    Thanks for the feedback! I have updated the patch, hope you can check it.
Best Regards
suyu.cmj

  9. Re: The same 2PC data maybe recovered twice

    Michael Paquier <michael@paquier.xyz> — 2023-07-18T05:13:52Z

    On Mon, Jul 17, 2023 at 05:20:00PM +0800, suyu.cmj wrote:
> Thanks for the feedback! I have updated the patch, hope you can check it.

I have looked at v3, and noticed that the stat() call is actually a
bit incorrect in its error handling because it would miss any errors
that happen when checking for the existence of the file.  The only
error that we should safely expect is ENOENT, for a missing entry.
All the other had better fail like the other code paths restoring 2PC
entries from the shared state.  At the end, I have made the choice of
relying only on access() to check the existence of the file as this is
an internal place, simplified a bit the comment.  Finally, I have made
the choice to remove the assert-only check.  After sleeping on it, it
would have value in very limited cases while a bunch of recovery cases
would take a hit, including developers with large 2PC setups, so there
are a lot of downsides with limited upsides.
--
Michael

  10. Re: The same 2PC data maybe recovered twice

    suyu.cmj <mengjuan.cmj@alibaba-inc.com> — 2025-07-09T01:57:08Z

    Hi, all. I would like to sync up on an issue I've recently encountered. Previously, the following logic was added to the PrepareRedoAdd() when fixing this 2PC recovery bug:
------------------------------------------------------------------
From:Michael Paquier <michael@paquier.xyz>
Send Time:2023 Jul. 18 (Tue.) 13:14
To:"蔡梦娟(玊于)"<mengjuan.cmj@alibaba-inc.com>
CC:"zhihui.fan1213"<zhihui.fan1213@gmail.com>; "pgsql-bugs"<pgsql-bugs@lists.postgresql.org>
Subject:Re: The same 2PC data maybe recovered twice
On Mon, Jul 17, 2023 at 05:20:00PM +0800, suyu.cmj wrote:
> Thanks for the feedback! I have updated the patch, hope you can check it.
I have looked at v3, and noticed that the stat() call is actually a
bit incorrect in its error handling because it would miss any errors
that happen when checking for the existence of the file. The only
error that we should safely expect is ENOENT, for a missing entry.
All the other had better fail like the other code paths restoring 2PC
entries from the shared state. At the end, I have made the choice of
relying only on access() to check the existence of the file as this is
an internal place, simplified a bit the comment. Finally, I have made
the choice to remove the assert-only check. After sleeping on it, it
would have value in very limited cases while a bunch of recovery cases
would take a hit, including developers with large 2PC setups, so there
are a lot of downsides with limited upsides.
--
Michael

  11. Re: The same 2PC data maybe recovered twice

    suyu.cmj <mengjuan.cmj@alibaba-inc.com> — 2025-07-09T05:51:03Z

    I'm sorry that the content of the previous email was incomplete. Let me continue with the issue I've recently encountered.
Previously, the following logic was added to the PrepareRedoAdd() function when fixing this 2PC recovery bug:
+ if (!XLogRecPtrIsInvalid(start_lsn))
+ {
+ char path[MAXPGPATH];
+
+ TwoPhaseFilePath(path, hdr->xid);
+
+ if (access(path, F_OK) == 0)
+ {
+ ereport(reachedConsistency ? ERROR : WARNING,
+ (errmsg("could not recover two-phase state file for transaction %u",
+ hdr->xid),
+ errdetail("Two-phase state file has been found in WAL record %X/%X, but thi
s transaction has already been restored from disk.",
+ LSN_FORMAT_ARGS(start_lsn))));
+ return;
+ }
+ }
Every time we add a 2PC from a WAL record, we check whether the corresponding 2PC file exists on the storage. When a consistent state is reached, it is highly likely that there will be no corresponding files on the storage, so it is probable that an empty file will be accessed. Each time a file is accessed, the OS creates a dentry for it and increases the reference count of the parent directory's dentry. The type of the dentry reference count is int32. When there are many 2PC transactions, this logic may lead to an overflow of the parent dentry's reference count. When reclaiming a dentry, such as during memory reclamation or when deleting the directory, the overflow of the reference count could ultimately cause the OS to crash.
I am considering whether the logic in this part can be modified as follows:
+ if (!XLogRecPtrIsInvalid(start_lsn) && !reachedConsistency)
+ {
+ char path[MAXPGPATH];
+
+ TwoPhaseFilePath(path, hdr->xid);
+
+ if (access(path, F_OK) == 0)
+ {
+ ereport(WARNING,
+ (errmsg("could not recover two-phase state file for transaction %u",
+ hdr->xid),
+ errdetail("Two-phase state file has been found in WAL record %X/%X, but thi
s transaction has already been restored from disk.",
+ LSN_FORMAT_ARGS(start_lsn))));
+ return;
+ }
+ }
Currently, since restoreTwoPhaseData() is the only function that restores 2PC transactions from disk before the recovery starts, after reaching a consistent state, the 2PC transactions are only added from the WAL. Under normal circumstances, there should not be any corresponding 2PC files on the storage at that point. Therefore, I prefer to perform the file access checks only when the server has not yet reached a consistent state. Once consistency has been reached, if a duplicate 2PC transaction is added, it will directly result in an error in the subsequent replay logic.
Do you think this modification is feasible? I look forward to your feedback
Best Regards
suyu.cmj
------------------------------------------------------------------
From:CAI, Mengjuan <mengjuan.cmj@alibaba-inc.com>
Send Time:2025 Jul. 9 (Wed.) 09:57
To:Michael Paquier<michael@paquier.xyz>
CC:"pgsql-bugs"<pgsql-bugs@lists.postgresql.org>
Subject:Re: The same 2PC data maybe recovered twice
Hi, all. I would like to sync up on an issue I've recently encountered. Previously, the following logic was added to the PrepareRedoAdd() when fixing this 2PC recovery bug:
------------------------------------------------------------------
From:Michael Paquier <michael@paquier.xyz>
Send Time:2023 Jul. 18 (Tue.) 13:14
To:"蔡梦娟(玊于)"<mengjuan.cmj@alibaba-inc.com>
CC:"zhihui.fan1213"<zhihui.fan1213@gmail.com>; "pgsql-bugs"<pgsql-bugs@lists.postgresql.org>
Subject:Re: The same 2PC data maybe recovered twice
On Mon, Jul 17, 2023 at 05:20:00PM +0800, suyu.cmj wrote:
> Thanks for the feedback! I have updated the patch, hope you can check it.
I have looked at v3, and noticed that the stat() call is actually a
bit incorrect in its error handling because it would miss any errors
that happen when checking for the existence of the file. The only
error that we should safely expect is ENOENT, for a missing entry.
All the other had better fail like the other code paths restoring 2PC
entries from the shared state. At the end, I have made the choice of
relying only on access() to check the existence of the file as this is
an internal place, simplified a bit the comment. Finally, I have made
the choice to remove the assert-only check. After sleeping on it, it
would have value in very limited cases while a bunch of recovery cases
would take a hit, including developers with large 2PC setups, so there
are a lot of downsides with limited upsides.
--
Michael

  12. Re: The same 2PC data maybe recovered twice

    Michael Paquier <michael@paquier.xyz> — 2025-07-09T07:35:23Z

    On Wed, Jul 09, 2025 at 01:51:03PM +0800, suyu.cmj wrote:
> Currently, since restoreTwoPhaseData() is the only function that
> restores 2PC transactions from disk before the recovery starts,
> after reaching a consistent state, the 2PC transactions are only
> added from the WAL. Under normal circumstances, there should not be
> any corresponding 2PC files on the storage at that point. Therefore,
> I prefer to perform the file access checks only when the server has
> not yet reached a consistent state. Once consistency has been
> reached, if a duplicate 2PC transaction is added, it will directly
> result in an error in the subsequent replay logic. 

There is a bit more going on in this code that needs to be fixed.

Please see the following thread, that also relates to the state of the
2PC recovery code when we read them from disk at the beginning of
recovery, with the bottom part being the most relevant:
https://www.postgresql.org/message-id/Z5sd5O9JO7NYNK-C%40paquier.xyz
--
Michael

  13. Re: The same 2PC data maybe recovered twice

    (unknown) — 2025-07-14T06:46:25Z

    Hi Michael,
Thank you for your reply. I reviewed the thread you mentioned, and it seems that the issue needing to be fixed is not the same as the one I previously raised.
I am considering whether the 2PC file check logic in PrepareRedoAdd() can be modified. Currently, each time a XLOG_XACT_PREPARE WAL entry is replayed, it checks for the corresponding 2PC file using access(). Each access operation creates a dentry in the OS, and in most cases, the file being accessed does not exist. When there are many 2PC transactions, this logic may lead to an increase in OS slab memory. In worse scenarios, it could cause the reference count of the parent directory's dentry to overflow, potentially leading to an OS crash. Typically, when accessing existing files, the disk will fill up before the dentry reference count overflows.
Therefore, I would like to propose a modification. Attached is my patch for your review, and I hope you can take a look at it.
Best regards,
suyu.cmj

  14. Re: The same 2PC data maybe recovered twice

    Michael Paquier <michael@paquier.xyz> — 2025-07-14T08:14:43Z

    On Mon, Jul 14, 2025 at 02:46:25PM +0800, CAI, Mengjuan wrote:
> Thank you for your reply. I reviewed the thread you mentioned, and
> it seems that the issue needing to be fixed is not the same as the
> one I previously raised.
> I am considering whether the 2PC file check logic in
> PrepareRedoAdd() can be modified. Currently, each time a
> XLOG_XACT_PREPARE WAL entry is replayed, it checks for the
> corresponding 2PC file using access(). Each access operation creates
> a dentry in the OS, and in most cases, the file being accessed does
> not exist. When there are many 2PC transactions, this logic may lead
> to an increase in OS slab memory. In worse scenarios, it could cause
> the reference count of the parent directory's dentry to overflow,
> potentially leading to an OS crash. Typically, when accessing
> existing files, the disk will fill up before the dentry reference
> count overflows. 
> Therefore, I would like to propose a modification. Attached is my
> patch for your review, and I hope you can take a look at it.

@@ -2520,8 +2520,16 @@ PrepareRedoAdd(FullTransactionId fxid, char *buf,
[...]
-	if (!XLogRecPtrIsInvalid(start_lsn))
+	if (!XLogRecPtrIsInvalid(start_lsn) && !reachedConsistency)

Actually, what you are doing is incorrect because we could miss some
ERRORs for example if a base backup was incorrect if come files were
present in pg_twophase?

It's not really true that what you are changing here has no
interaction with the beginning of recovery, the other thread is about
the fact that reading the 2PC files from disk when !reachedConsistency
is a bad concept that we should avoid, impacting the assumption the
code path you are changing here relies on.  At the end, it may be
possible that we're able to remove this check entirely..
--
Michael

  15. Re: The same 2PC data maybe recovered twice

    suyu.cmj <mengjuan.cmj@alibaba-inc.com> — 2025-07-14T09:57:18Z

    > Actually, what you are doing is incorrect because we could miss some
> ERRORs for example if a base backup was incorrect if come files were
> present in pg_twophase?
Yes, you are right, base backup with incorrect files would be a problem.
> It's not really true that what you are changing here has no
> interaction with the beginning of recovery, the other thread is about
> the fact that reading the 2PC files from disk when !reachedConsistency
> is a bad concept that we should avoid, impacting the assumption the
> code path you are changing here relies on. At the end, it may be
> possible that we're able to remove this check entirely..
Is there any recent discussion or plan regarding this? I noticed that in the latest code branch, 2PC data on disk is still being restored when !reachedConsistency. If there are any updates or discussions, I would greatly appreciate it if you could let me know
Best regards