Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix handling of orphaned 2PC files in the future at recovery

  1. An improvement of ProcessTwoPhaseBuffer logic

    Vitaly Davydov <v.davydov@postgrespro.ru> — 2024-12-24T13:26:32Z

    Dear Hackers,

I would like to discuss ProcessTwoPhaseBuffer function. It reads two-phase transaction states from disk or the WAL. It takes xid as well as some other input parameters and executes the following steps:

Step #1: Check if xid is committed or aborted in clog (TransactionIdDidCommit, TransactionIdDidAbort)
Step #2: Check if xid is not equal or greater than ShmemVariableCache->nextXid
Step #3: Read two-phase state for the specified xid from memory or the corresponding file and returns it

In some, very rare scenarios, the postgres instance will newer recover because of such logic. Imagine, that the two_phase directory contains some files with two-phase states of transactions of distant future. I assume, it can happen if some WAL segments are broken and ignored (as well as clog data) but two_phase directory was not broken. In recovery, postgresql reads all the files in two_phase and tries to recover two-phase states.

The problem appears in the functions TransactionIdDidCommit or TransactionIdDidAbort. These functions may fail with the FATAL message like below when no clog state on disk is available for the xid:

FATAL:  could not access status of transaction 286331153
DETAIL:  Could not open file "pg_xact/0111": No such file or directory.

Such error do not allow the postgresql instance to be started.

My guess, if to swap Step #1 with Step #2 such error will disappear because transactions will be filtered when comparing xid with ShmemVariableCache->nextXid before accessing clog. The function will be more robust. In general, it works but I'm not sure that such logic will not break some rare boundary cases. Another solution is to catch and ignore such error, but the original solution is the simpler one. I appreciate any thoughts concerning this topic. May be, you know some cases when such change in logic is not relevant?

Thank you in advance!

With best regards,
Vitaly

  2. Re: An improvement of ProcessTwoPhaseBuffer logic

    Michael Paquier <michael@paquier.xyz> — 2024-12-25T05:04:40Z

    On Tue, Dec 24, 2024 at 04:26:32PM +0300, Vitaly Davydov wrote:
> In some, very rare scenarios, the postgres instance will newer
> recover because of such logic. Imagine, that the two_phase directory
> contains some files with two-phase states of transactions of distant
> future. I assume, it can happen if some WAL segments are broken and
> ignored (as well as clog data) but two_phase directory was not
> broken. In recovery, postgresql reads all the files in two_phase and
> tries to recover two-phase states.

Well, one recent issue in this area is a bug fixed by cf4401fe6cf5:
https://www.postgresql.org/message-id/tencent_A7F059B5136A359625C7B2E4A386B3C3F007@qq.com

If you see other bug patterns like this one, please let me know.
There are good changes that I could be involved in stuff that touched
this area of the code, so I'm mostly behind its maintenance these
days.

> My guess, if to swap Step #1 with Step #2 such error will disappear
> because transactions will be filtered when comparing xid with
> ShmemVariableCache->nextXid before accessing clog. The function will
> be more robust. In general, it works but I'm not sure that such
> logic will not break some rare boundary cases. Another solution is
> to catch and ignore such error, but the original solution is the
> simpler one. I appreciate any thoughts concerning this topic. May
> be, you know some cases when such change in logic is not relevant? 

Hmm.  Historically, up to 9.6, StandbyRecoverPreparedTransactions()
and RecoverPreparedTransactions() have been in charge of doing the
commit/abort checks with potentially clog lookups when it came to past
files.  PrescanPreparedTransactions() has been in charge of doing the
check about future files.

If you look closely, PrescanPreparedTransactions() is called in two
code paths before StandbyRecoverPreparedTransactions(), meaning that
we did the checks for the future files first, then the past checks
with potential CLOG lookups. 

In v10~, things have changed to use ProcessTwoPhaseBuffer(), and as
you say, the checks are what we have now: they're done in a reverse
order.  So, yes, I agree that you have a point here: the past code was
safer because it would have been able to avoid clog lookups if we had
a 2PC file in the future for a reason or another so it was safer than
what we have now.

I'm wondering if the attached patch is something worth backpatching,
actually, as these failing CLOG lookups can lead to spurious failures
at startup.  HEAD-only would be OK based on the odds so that's a
no-brainer.  Vitaly, have you seen that in the wild as an effect of
future 2PC files?

Any opinions from others?
--
Michael