Call rm_redo in a temporary memory context
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.
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:t50078psql -h localhost -U postgresBuilt from patchset v1 (message #1), July 27, 2026 at 11:52 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 t50078_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 t50078_1 && git checkout t50078_1Patchset v1 (message #1) is on t50078_1
Many resource managers set up a temporary memory context which is reset
after replaying the record. It seems a bit silly for each rmgr to do
that on their own, so I propose that we do it in a centralized fashion.
The attached patch creates one new temporary context and switches to it
for each rm_redo() call.
I was afraid of the overhead of the MemoryContextReset between each WAL
record since this is a very hot codepath, but it doesn't seem to be
noticeable. I used the attached scripts to benchmark it.
redobench-setup.sh sets up a base backup and about 5 GB of WAL. The WAL
consists of just tiny logical decoding messages, no real page
modifications. The idea is that replaying that WAL should make any
per-record overhead stand out as much as possible, since there's no real
work to do. Use redobench.sh to perform the replay. I am not seeing any
measurable difference this patch, so I think we're good. But if we
needed to optimize, we could e.g. have an inlined fastpath version of
MemoryContextReset for the common case that the context is empty, or
only reset it every 100 records or something.
This leaves no built-in rmgrs with any rm_startup or rm_clenaup hooks.
Extensions might still use them, and they seem like they might be
useful, so I kept them.
There was no natural place to document this, so I added a brief
explanation of rm_redo in the RmgrData comment, and then tacked the
information about the memory context there too. I also added a note in
"Custom WAL Resource Managers" section of the docs to point out that
this changed in v18.
(Why am I doing this now? I was browsing through all the global
variables for the multithreading work, and these "opCtx"s caught my eye.
This is in no way critical for multithreading though.)
--
Heikki Linnakangas
Neon (https://neon.tech)
Hi!
On Wed, 7 Aug 2024 at 16:24, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
Many resource managers set up a temporary memory context which is reset
after replaying the record. It seems a bit silly for each rmgr to do
that on their own, so I propose that we do it in a centralized fashion.
The attached patch creates one new temporary context and switches to it
for each rm_redo() call.I was afraid of the overhead of the MemoryContextReset between each WAL
record since this is a very hot codepath, but it doesn't seem to be
noticeable. I used the attached scripts to benchmark it.
redobench-setup.sh sets up a base backup and about 5 GB of WAL. The WAL
consists of just tiny logical decoding messages, no real page
modifications. The idea is that replaying that WAL should make any
per-record overhead stand out as much as possible, since there's no real
work to do. Use redobench.sh to perform the replay. I am not seeing any
measurable difference this patch, so I think we're good. But if we
needed to optimize, we could e.g. have an inlined fastpath version of
MemoryContextReset for the common case that the context is empty, or
only reset it every 100 records or something.This leaves no built-in rmgrs with any rm_startup or rm_clenaup hooks.
Extensions might still use them, and they seem like they might be
useful, so I kept them.There was no natural place to document this, so I added a brief
explanation of rm_redo in the RmgrData comment, and then tacked the
information about the memory context there too. I also added a note in
"Custom WAL Resource Managers" section of the docs to point out that
this changed in v18.(Why am I doing this now? I was browsing through all the global
variables for the multithreading work, and these "opCtx"s caught my eye.
This is in no way critical for multithreading though.)--
Heikki Linnakangas
Neon (https://neon.tech)
+1 on the idea, since this simplifies RMGR API for extension developers.
Compiler warns about `src/backend/access/transam/xlogrecovery.c:1860`,
where we switch to maybe-uninitialized memory context. Lets assign
this to something.
--
Best regards,
Kirill Reshke
On Tue, Oct 01, 2024 at 03:29:19PM +0500, Kirill Reshke wrote:
Compiler warns about `src/backend/access/transam/xlogrecovery.c:1860`,
where we switch to maybe-uninitialized memory context. Lets assign
this to something.
+ yourself e.g. in <varname>rm_startup</varname>. Starting with version
+ 18.0, it's called in a temporary memory context instead, and if you need
+ to retain data over calls, you need to explicitly allocate them in
+ <literal>TopMemoryContext</literal>.Hmm. Is it a good idea to encourage that? This would be allocated in
the memory context where RmgrStartup() is called. For HEAD, it does
not matter because this is called only at the beginning of recovery in
the startup process. However, could it be a problem for out-of-core
code that wants to do a full SGMR startup and may want to rely on
something else than TopMemoryContext because the current code makes
memory context cleanup easier in the event if an error?
--
Michael