On Thu, Sep 28, 2023 at 9:46 AM Heikki Linnakangas <hlinn...@iki.fi> wrote: > Here's a new version of these patches. I fixed one comment and ran > pgindent, no other changes.
> Subject: [PATCH v2 1/3] Clarify the checks in RegisterBackgroundWorker. LGTM. I see it passes on CI, and I also tested locally with EXEC_BACKEND, with shared_preload_libraries=path/to/pg_prewarm.so which works fine. > Subject: [PATCH v2 2/3] Allocate Backend structs in PostmasterContext. LGTM. I checked that you preserve the behaviour on OOM (LOG), and you converted free() to pfree() in code that runs in the postmaster, but dropped it in the code that runs in the child because all children should delete PostmasterContext, making per-object pfree redundant. Good. > Subject: [PATCH v2 3/3] Fix misleading comment on StartBackgroundWorker(). LGTM. Hmm, maybe I would have called that function "BackgroundWorkerMain()" like several other similar things, but that's not important. This doesn't quite fix the problem I was complaining about earlier, but it de-confuses things. (Namely that if BackgroundWorkerList weren't a global variable, RegisterWorkerMain() wouldn't be able to find it, and if it took some kind of context pointer as an argument, _PG_init() functions wouldn't be able to provide it, unless we changed _PG_init() to take an argument, which we can't really do. Oh well.)