Re: An improvement of ProcessTwoPhaseBuffer logic

Vitaly Davydov <v.davydov@postgrespro.ru>

From: "Vitaly Davydov" <v.davydov@postgrespro.ru>
To: "Michael Paquier" <michael@paquier.xyz>
Cc: "pgsql-hackers" <pgsql-hackers@postgresql.org>
Date: 2024-12-25T16:18:18Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Integrate FullTransactionIds deeper into two-phase code

  2. Merge copies of converting an XID to a FullTransactionId.

  3. Revert recent changes related to handling of 2PC files at recovery

  4. Fix failures with incorrect epoch handling for 2PC files at recovery

Hi Michael,

Thank you for the explanation and the patch! I'm happy, that I seem to be on the right way.

On Wednesday, December 25, 2024 08:04 MSK, Michael Paquier <michael@paquier.xyz> wrote:
> Vitaly, have you seen that in the wild as an effect of future 2PC files?

I haven't heard about this problem in production, but I've encountered it when I did some development in two-phase functionality. I fixed it by swapping the if blocks and it solved my problem.

It is pretty easy to reproduce it on the master branch:

1. Start an instance with enabled prepared transactions
2. Create a simple prepared transaction
3. Do checkpoint to write the transaction two-phase state into a file in pg_twophase subdirectory
4. Copy the created file, change its name to reflect a future xid (in my case: cp 00000000000002E8 00000000000FF2E8)
5. Commit the prepared transaction
6. Stop the instance with -m immediate
7. Start the instance

After starting, you can get an error like "could not start server". In the log file you can find a message like: 

LOG:  database system was not properly shut down; automatic recovery in progress
FATAL:  could not access status of transaction 1045224
DETAIL:  Could not read from file "pg_xact/0000" at offset 253952: read too few bytes.
2024-12-25 18:38:30.606 MSK [795557] LOG:  startup process (PID 795560) exited with exit code 1

I tried your patch and it seems the server is started successfully. But I've found another problem in my synthetic test - it can not remove the file with the following message:

LOG:  database system was not properly shut down; automatic recovery in progress
WARNING:  removing future two-phase state file for transaction 1045224
WARNING:  could not remove file "pg_twophase/FFFFFFFF000FF2E8": No such file or directory
LOG:  redo starts at 0/1762978

The fill will never be removed automatically.
I guess, it is because we incorrectly calculate the two-phase file name using TwoPhaseFilePath in RemoveTwoPhaseFile in this scenario. It can be fixed if to pass file path directly from RecoverPreparedTransactions or StandbyRecoverPreparedTransaction into ProcessTwoPhaseBuffer -> RemoveTwoPhaseFile. I did it in the proposed patch https://www.postgresql.org/message-id/cedbe-65e0c000-1-6db17700%40133269862 (it is incomplete now).

With best regards,
Vitaly