Racing in ROLLBACK PREAPRED and checkpoint
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t254434psql -h localhost -U postgresBuilt from patchset v1 (message #1), November 02, 2026 at 04:24 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t254434_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t254434_1 && git checkout t254434_1Patchset v1 (message #1) is on t254434_1
Hi,
RecordTransactionAbortPrepared() does not set DELAY_CHKPT_START, unlike
the commit path. So a prepared transaction that was rolled back can come
back after a crash:
1. ROLLBACK PREPARED writes and flushes its abort record.
2. A checkpoint runs with its redo pointer after that record. pg_xact
does not show the abort yet, so the state file is kept on disk.
3. ROLLBACK PREPARED sets the abort in pg_xact and removes the state
file.
4. The OS crashes before that removal is durable.
5. Recovery starts after the abort record. It finds the state file and
pg_xact says nothing, so the transaction is prepared again.
6. COMMIT PREPARED now succeeds.
Then the WAL will have ROLLBACK and CoMMIT for the same xid.
Standbys and already saw the abort in step 1. After
step 6, a standby with assertions fails an assert in clog.c and stops.
TRAP: failed Assert("curval == 0 || (curval ==
TRANSACTION_STATUS_SUB_COMMITTED && status !=
TRANSACTION_STATUS_IN_PROGRESS) || curval == status")
Without assertions it silently marks the transaction committed.
The fix is to add DELAY_CHKPT_START same as the commit side.
The commit side has had this since the two-phase commit went in. The abort
side was written like a regular abort, and its comment says the pg_xact
update "is not absolutely necessary". That is true for a regular abort,
but here the state file can bring the transaction back.
This needs to be backported to 14.
It feels like these WAL, file change operations are all at risk, mutlixact
may also have the same issue, it is late here, I will check it tomorrow.
Thanks,
Shihao
Attachments:
t254434_1v1-0001-Delay-checkpoint-start-in-RecordTransactionAbortP.patchapplication/octet-stream; name=v1-0001-Delay-checkpoint-start-in-RecordTransactionAbortP.patchDownload+15-3
v1-0002-Add-test-for-checkpoint-during-ROLLBACK-PREPARED.patchapplication/octet-stream; name=v1-0002-Add-test-for-checkpoint-during-ROLLBACK-PREPARED.patchDownload+99-1
On Thu, Oct 29, 2026 at 12:00:56AM -0400, shihao zhong wrote:
The commit side has had this since the two-phase commit went in. The abort
side was written like a regular abort, and its comment says the pg_xact
update "is not absolutely necessary". That is true for a regular abort,
but here the state file can bring the transaction back.This needs to be backported to 14.
As far as I can see, that's pretty old. Dragging into the history of
the code, this issue is at least as old as 9c9b619473cd where the
inCommit flag has been introduced in replacement for the LWLocks.
It's as old as the introduction of 2PC back in 2005.
I'll spend a bit more time on that today, but your suggestion of just
relying on delayChkptFlags should do it, like the COMMIT PREPARED
path.
--
Michael
Hi Michael,
Thanks for looking into this.
I went through the other places that write a WAL record and then
change state races with the checkpointer. I found three more cases
and reported them in separate threads.
One is multixact creation [1]/messages/by-id/CAGRkXqSuiJ_X4+Z6rsD42pcuACnD9PhoQ-Os+mUJNmJarBWkNg@mail.gmail.com.
The other two are CLOG truncation and hash index bucket allocation [2]/messages/by-id/CAGRkXqTSAb-H4oxk=J9QBv-Kh2-o=cnCWuiSduMVaVzrKYmKXQ@mail.gmail.com.
The other paths I checked are safe.
[1]: /messages/by-id/CAGRkXqSuiJ_X4+Z6rsD42pcuACnD9PhoQ-Os+mUJNmJarBWkNg@mail.gmail.com
/messages/by-id/CAGRkXqSuiJ_X4+Z6rsD42pcuACnD9PhoQ-Os+mUJNmJarBWkNg@mail.gmail.com
[2]: /messages/by-id/CAGRkXqTSAb-H4oxk=J9QBv-Kh2-o=cnCWuiSduMVaVzrKYmKXQ@mail.gmail.com
/messages/by-id/CAGRkXqTSAb-H4oxk=J9QBv-Kh2-o=cnCWuiSduMVaVzrKYmKXQ@mail.gmail.com
Thanks,
Shihao
On Sun, Nov 01, 2026 at 10:37:16PM -0500, shihao zhong wrote:
Thanks for looking into this.
At the end I have no objection to setting up the checkpoint start flag
to close this race, so applied down to v14. I have reworded quite a
bit the comments, though, to mention a little bit more the
similarities with the COMMIT PREPARED path and somewhat the regular
COMMIT path, as these relate as well to the pg_xact updates. The
difference here is that the file is on disk.
One is multixact creation [1].
The other two are CLOG truncation and hash index bucket allocation [2].
I am not sure of I will be able to look at any of them, but perhaps
others will be able to chime in.
Another thing that is worth mentioning here is that we have more
problems around 2PC and recovery setups. See the reason behind
a6c70f68cdeb and its thread:
/messages/by-id/20250116010051.f3.nmisch@google.com
There is a deeper design issue in this area of the code where we have
the idea to look at the CLOGs for the commit or aborted states of the
2PC transactions without having reached a consistent state (both crash
and base backup recovery included). In this case the CLOG data may be
inconsistent.
It's different than what you have reported here: it's not OK to have a
checkpoint discard data we are going to need to decide if a 2PC
transaction has been aborted or not.
While the test is useful to reproduce the issue, I was not sure that
it is mandatory to run it for such a short race window, so I have left
it out of the equation. If anybody has else has an opinion to offer
about that, feel free.
--
Michael