| From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
|---|---|
| To: | "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Refactor recovery conflict signaling a little |
| Date: | 2026-01-22 23:20:20 |
| Message-ID: | 4cc13ba1-4248-4884-b6ba-4805349e7f39@iki.fi |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
I had a look at recovery conflict signaling and a few things caught my
eye. No functional changes, but some cleanups and readability improvements:
Patch 0001: Remove useless errdetail_abort()
--------------------------------------------
The function is supposed to add DETAIL to errors when you are in an
aborted transaction, if the transaction was aborted by a recovery
conflict, like this:
ERROR: current transaction is aborted, commands ignored until end of
transaction block"
DETAIL: Abort reason: recovery conflict
But I don't see how to reach that. If a transaction is aborted by
recovery conflict, you get a different error like this:
ERROR: canceling statement due to conflict with recovery
DETAIL: User was holding a relation lock for too long.
The transaction abort clears the 'recoveryConflictPending' flag, so even
if that happens in a transaction block, you don't get that "DETAIL:
Abort reason: recovery conflict" in the subsequent errors.
errdetail_abort() was introduced in commit a8ce974cdd. I suppose it was
needed back then, but the signal handling has changed a lot since.
Looking at that commit now, though, I don't really understand how it was
reachable even back then. (Except with a race with an unrelated
transaction abort, see commit message)
Has anyone seen the "DETAIL: Abort reason: recovery conflict" in recent
years, or ever? If not, let's rip it out.
0002: Don't hint that you can reconnect when the database is dropped
--------------------------------------------------------------------
If you're connected to a database is being dropped, during recovery, you
get an error like this:
FATAL: terminating connection due to conflict with recovery
DETAIL: User was connected to a database that must be dropped.
HINT: In a moment you should be able to reconnect to the database and
repeat your command.
The hint seems misleading. The database is being dropped, you most
likely can *not* reconnect to it. Let's remove it.
0003-0004: Separate RecoveryConflictReasons from procsignals
-------------------------------------------------------------
We're currently using different PROCSIG_* flags to indicate different
kinds of recovery conflicts. We're also abusing the same flags in
functions like LogRecoveryConflict, which isn't related to inter-process
signaling. It seems better to have a separate enum for the recovery
conflict reasons. With this patch, there's just a single
PROCSIG_RECOVERY_CONFLICT to wake up a process on a recovery conflict,
and the reason is communicated by setting a flag in a bitmask in PGPROC.
I was inspired to do this in preparation of my project to replaces
latches with "interrupts". By having just a single PROCSIG flag, we
reduce the need for "interrupt bits" with that project. But it seems
nicer on its own merits too.
0005: Refactor ProcessRecoveryConflictInterrupt for readability
---------------------------------------------------------------
The function had a switch-statement with fallthrough through all the
cases. It took me a while to understand how it works. Once I finally
understood it, I refactored it to not rely on the fallthrough. I hope
this makes it easier for others too.
- Heikki
| Attachment | Content-Type | Size |
|---|---|---|
| 0001-Remove-useless-errdetail_abort.patch | text/x-patch | 11.9 KB |
| 0002-Don-t-hint-that-you-can-reconnect-when-the-database-.patch | text/x-patch | 2.0 KB |
| 0003-Use-ProcNumber-rather-than-pid-in-ReplicationSlot.patch | text/x-patch | 11.1 KB |
| 0004-Separate-RecoveryConflictReasons-from-procsignals.patch | text/x-patch | 37.7 KB |
| 0005-Refactor-ProcessRecoveryConflictInterrupt-for-readab.patch | text/x-patch | 15.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-01-22 23:23:11 | Re: Remove PG_MMAP_FLAGS |
| Previous Message | Michael Paquier | 2026-01-22 23:12:49 | Re: Fix rounding method used to compute huge pages |