Re: An improvement of ProcessTwoPhaseBuffer logic

It seems, there are much deeper problems with twophase transactions as I thought. I'm interested in fixing twophase transactions, because I support a solution which actively uses twophase transactions. I'm interested to get more deeply into the twophase functionality. Below, I just want to clarify some ideas behind twophase transactions. I appreciate, if you comment my point of view, or just ignore this email if you find it too naive and boring.

Two phase files are created after checkpoint to keep twophase states on disk after WAL truncation. For transactions, that are inside the checkpoint horizon, we do not create two phase files because the states are currently stored in the WAL.

Based on the thesis above, I guess, we have to read only those twophase files which are related to the transactions before the latest checkpoint. Its full xid should be lesser than TransamVariables->nextXid (which is the same as ControlFile->checkPointCopy.nextXid at the moment of StartupXLOG -> restoreTwoPhaseData call). The files with greater (or equal) full xids, should be ignored and removed. That's all what we need in restoreTwoPhaseData, I believe.

In the current implementation, such check is applied in restoreTwoPhaseData -> ProcessTwoPhaseBuffer but after checking for xid in CLOG. I'm not sure, why we check CLOG here. Once we truncate the WAL on checkpoint and save twophase states into pg_twophase, these files must store states of real transactions from past. I mean, if someone creates a stub file with full xid < TransamVariables->nextXid, we have no means (except CLOG ?) to check that this file belongs to a real transaction from past. CLOG check seems to be a weak attempt to deal with it. At this point, I'm not sure that CLOG may contain states for all full xids of existing twophase files. I guess, we should call restoreTwoPhaseData at start of recovery but we shouldn't check CLOG at this stage. May be it is reasonable to check some not so old xids which are greater than the current CLOG horizon, but I'm not sure how CLOG segments are managed and how the horizon is moving.

There is another question about the loading order of twophase files but I think it doesn't matter in which order we load these files. But I would prefer to load it in full xid ascending order.

On Tuesday, January 28, 2025 08:02 MSK, Michael Paquier <michael@paquier.xyz> wrote:

> Noah's
> proposal at [1] is much closer to the long-term picture that would
> look adapted.
> - The CLOG lookups that can happen in ProcessTwoPhaseBuffer() during
> recovery while a consistent state is not reached are still possible
> (planning to start a different thread about this specific issue).
> 
> [1]: https://www.postgresql.org/message-id/20250116205254.65.nmisch@google.com

Agree, thank you, but my simple patch with some adjustments and swapping of checks in ProcessTwoPhaseBuffer may be back-ported. It doesn't fix all the problems but may help to fix the problem with twophase files related to broken latest WAL segments.

With best regards,
Vitaly