Re: An improvement of ProcessTwoPhaseBuffer logic

On Wed, Jan 22, 2025 at 03:45:30PM +0300, Vitaly Davydov wrote:
> I hope you may find this patch worth to consider. You may consider
> this patch as one more step to a complete migration to
> FullTransactionId in twophase.c for 17+ versions.

Heavy refactorings are only going to be something for 18~, as far as
my analysis goes.

> I assume that if a xid is coming from in-memory states, it has to be
> in the current xid range. Based on this assumption, I would conclude
> that if the xid is coming via the interface (external) functions
> which are defined in twophase.h, AdjustToFullTransactionId can be
> applied to such xid. 
> 
> There is another story when we define xid from two phase file names,
> when reading pg_twophase directory. In 17+ twophase file names was
> changed to contain tx epoch as well. Once we work with twophase
> files, we have to use full xids. Function AdjustToFullTransactionId
> is not applicable in this case, because pg_twophase directory may
> contain any garbage files with future or past full xids which are
> outside of the current range.
> 
> Based on these assumptions (AdjustToFullTransactionId or use full
> xids) some other functions were modified as shown in the patch.

Yeah, we need to do something here.  Anyway, I don't think what you
are posting here is ambitious enough for two reasons:
- We should try to limit the number of times where we do maths between
TransactionId and FullTransactionId.  It's going to be cleaner if we
make the 2PC interface require FullTransactionIds all the time (note
that the point of storing a fxid or an xid is different).  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
--
Michael