From: "Vitaly Davydov" <v.davydov@postgrespro.ru>
In-Reply-To: <Z5hk87LhCu3EKA90@paquier.xyz>
Content-Type: text/plain; charset="utf-8"
References: <16aa85-676ec500-3d-23db9b40@59039559>
 <Z3Hyj3kvkj3b9dFi@paquier.xyz>
 <20250116010051.f3.nmisch@google.com>
 <Z4i6MbUlZxjK1rRh@paquier.xyz>
 <20250116205254.65.nmisch@google.com>
 <20250117005221.05.nmisch@google.com>
 <Z4m6k--M1AvAWuRF@paquier.xyz>
 <20250117024416.8c.nmisch@google.com>
 <Z4nLfw6hcdz0BTkP@paquier.xyz>
 <3ebed0-6790e880-3-24447380@20764243> <Z5hk87LhCu3EKA90@paquier.xyz>
Date: Wed, 29 Jan 2025 15:00:54 +0300
To: "pgsql-hackers" <pgsql-hackers@postgresql.org>
MIME-Version: 1.0
Message-ID: <96951-679a1880-17-e255290@36905887>
Subject: =?utf-8?q?Re=3A?= An improvement of ProcessTwoPhaseBuffer logic
User-Agent: SOGoMail 5.11.2
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/96951-679a1880-17-e255290%4036905887>
Precedence: bulk

It seems, there are much deeper problems with twophase transactions as =
I thought. I'm interested in fixing twophase transactions, because I su=
pport a solution which actively uses twophase transactions. I'm interes=
ted to get more deeply into the twophase functionality. Below, I just w=
ant to clarify some ideas behind twophase transactions. I appreciate, i=
f you comment my point of view, or just ignore this email if you find i=
t 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 check=
point 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 checkpoi=
nt. Its full xid should be lesser than TransamVariables->nextXid (which=
 is the same as ControlFile->checkPointCopy.nextXid at the moment of St=
artupXLOG -> restoreTwoPhaseData call). The files with greater (or equa=
l) 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 restoreTwoPhase=
Data -> ProcessTwoPhaseBuffer but after checking for xid in CLOG. I'm n=
ot sure, why we check CLOG here. Once we truncate the WAL on checkpoint=
 and save twophase states into pg=5Ftwophase, these files must store st=
ates of real transactions from past. I mean, if someone creates a stub =
file with full xid < TransamVariables->nextXid, we have no means (excep=
t CLOG ?) to check that this file belongs to a real transaction from pa=
st. CLOG check seems to be a weak attempt to deal with it. At this poin=
t, I'm not sure that CLOG may contain states for all full xids of exist=
ing twophase files. I guess, we should call restoreTwoPhaseData at star=
t of recovery but we shouldn't check CLOG at this stage. May be it is r=
easonable to check some not so old xids which are greater than the curr=
ent CLOG horizon, but I'm not sure how CLOG segments are managed and ho=
w 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 wo=
uld prefer to load it in full xid ascending order.

On Tuesday, January 28, 2025 08:02 MSK, Michael Paquier <michael@paquie=
r.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).
>=20
> [1]: https://www.postgresql.org/message-id/20250116205254.65.nmisch@g=
oogle.com

Agree, thank you, but my simple patch with some adjustments and swappin=
g 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 r=
elated to broken latest WAL segments.

With best regards,
Vitaly