On Tue, Aug 27, 2013 at 9:50 AM, Andres Freund wrote:
BgwHandleStatus GetBackgroundWorkerPid(BackgroundWorkerHandle *handle, pid_t *pid);
BgwHandleStatus WaitForBackgroundWorkerStartup(BackgroundWorkerHandle *handle, pid_t *pid);
OK, here's a patch that API. I renamed the constants a bit, because a
process that has stopped is not necessarily gone; it could be
configured for restart. But we can say that it is stopped, at the

I'm not sure that this API is an improvement. But I think it's OK, if
you prefer it.
Thanks, looks more readable to me. I like code that tells me what it
does without having to look in other places ;)
Well, fair enough. But you might have to look around a bit to figure
out that you now have two functions each of which can return 3 out of
a possible 4 values. But whatever.
+ <function>RegisterDynamicBackgroundWorker(<type>BackgroundWorker
+ *worker, BackgroundWorkerHandle **handle</type>)</function>. Unlike
+ <function>RegisterBackgroundWorker</>, which can only be called from within
+ the postmaster, <function>RegisterDynamicBackgroundWorker</function> must be
+ called from a regular backend.
s/which can only be called from within the posmaster/during initial
startup, via shared_preload_libraries/?
This seems like a possible doc clarification not intimately related to
the patch at hand. The only reason that paragraph is getting reflowed
is because of the additional argument to
RegisterDynamicBackgroundWorker(). On the substance, I could go
either way on whether your proposed text is better than what's there
now. It seems like it might need a bit of wordsmithing, anyway.
Good catch.
-RegisterDynamicBackgroundWorker(BackgroundWorker *worker)
+RegisterDynamicBackgroundWorker(BackgroundWorker *worker,
+ BackgroundWorkerHandle **handle)
Hm. Not this patches fault, but We seem to allow bgw_start_time ==
BgWorkerStart_PostmasterStart here which doesn't make sense...
I can add a check for that. I agree that it's a separate patch.
Maybe add something like Assert(hanlde->slot < max_worker_processes);? Sure.
Shouldn't that loop have a CHECK_FOR_INTERRUPTS()? Yep.
Theoretically this would unset set_latch_on_sigusr1 if it was previously
already set to 'true'. If you feel defensive you could safe the previous
Probably a good plan.
So, besides those I don't see much left to be done. I haven't tested
EXEC_BACKEND, but I don't anything specific to that here.
I certainly can't promise that the code is bug-free. But I think it's
probably better to get this into the tree and let people start playing
around with it than to continue to maintain it in my private sandbox.
At this point it's just infrastructure anyway, though there seems to
be a good deal of interest in it from more than just myself... and I
do think it's a useful step forward from where we are today.

Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Search Discussions

Discussion Posts


Follow ups

Related Discussions

Discussion Navigation
viewthread | post
posts ‹ prev | 14 of 17 | next ›
Discussion Overview
grouppgsql-hackers @
postedJul 24, '13 at 4:46p
activeAug 28, '13 at 6:44p



site design / logo © 2018 Grokbase