On Fri, Aug 27, 2021 at 10:02 AM Michael Paquier <mich...@paquier.xyz> wrote: > > On Fri, Aug 27, 2021 at 09:34:24AM +0800, Julien Rouhaud wrote: > > That shouldn't lead to any problem right? > > Well, bgworker_should_start_now() does not exist for that, and > RegisterBackgroundWorker() is the one doing the classification job, so > it would be more consistent to keep everything under control in the > same code path.
I'm not sure it's that uncontroversial. The way I see RegisterBackgroundWorker() is that it's responsible for doing some sanity checks to see if the module didn't make any error and if ressources are available. Surely checking for IsBinaryUpgrade should not be the responsibility of third-party code, so the question is whether binary upgrade is seen as a resource and as such a reason to forbid bgworker registration, in opposition to forbid the launch itself. Right now AFAICT there's no official API to check if a call to RegisterBackgroundWorker() succeeded or not, but an extension could check by itself using BackgroundWorkerList in bgworker_internals.h, and error out or something if it didn't succeed, as a way to inform users that they didn't configure the instance properly (like maybe increasing max_worker_processes). Surely using a *_internals.h header is a clear sign that you expose yourself to problems, but adding an official way to check for bgworker registration doesn't seem unreasonable to me. Is that worth the risk to have pg_upgrade erroring out in this kind of scenario, or make the addition of a new API to check for registration status more difficult?
