Hi! On Wed, 7 Aug 2024 at 16:24, Heikki Linnakangas <hlinn...@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
