Horton

Re: An improvement of ProcessTwoPhaseBuffer logic

Vitaly Davydov <v.davydov@postgrespro.ru>

View thread
From: "Vitaly Davydov" <v.davydov@postgrespro.ru>
To: "Michael Paquier" <michael@paquier.xyz>
Cc: "pgsql-hackers" <pgsql-hackers@postgresql.org>
Date: 2024-12-27T15:16:24Z
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

Attachments

On Friday, December 27, 2024 10:37 MSK, Michael Paquier <michael@paquier.xyz> wrote:

> So please see the attached.  You will note that RemoveTwoPhaseFile(),
> ProcessTwoPhaseBuffer() and TwoPhaseFilePath() now require a
> FullTransactionId, hence callers need to think about the epoch to use.
> That should limit future errors compared to passing the file name as
> optional argument.

In general, I like your solution to use FullTransactionId. I haven't found any evident problems. I just would like to propose to create a couple of new functions like RemoveTwoPhaseFileInCurrentEpoch, TwoPhaseFilePathInCurrentEpoch which accept TransactionId instead FullTransactionId. It may make the code clearer and result into less boilerplate code. I tried to do some hand testing. It seems, the problem is gone with the patch. Thank you!

As an idea, I would like to propose to store FullTransactionId in global transaction state instead of TransactionId. I'm not sure, it will consume significant additional memory, but it make the code more clear and probably result into less number of locks.

With best regards,
Vitaly