Itagaki-san,
thanks for reviewing this.
On 08/26/2010 03:39 AM, Itagaki Takahiro wrote:
> Other changes in the patch doesn't seem be always needed for the purpose.
> In other words, the patch is not minimal.
Hm.. yeah, maybe the separation between step1 and step2 is a bit
arbitrary. I'll look into it.
> The original purpose could be done without IMessage.
Agreed, that's the one exception. I've mentioned why that is and I don't
currently feel like coding an unneeded variant which doesn't use imessages.
> Also, min/max_spare_background_workers are not used in the patch at all.
You are right, it only starts to get used in step2, so the addition
should probably move there, right.
> (BTW, min/max_spare_background_*helpers* in postgresql.conf.sample is
> maybe typo.)
Uh, correct, thank you for pointing this out. (I've originally named
these helper processes before. Merging with autovacuum, it made more
sense to name them background *workers*)
> The most questionable point for me is why you didn't add any hook functions
> in the coordinator process.
Because I'm a hook-hater. ;-)
No, seriously: I don't see what problem hooks could have solved. I'm
coding in C and extending the Postgres code. Deciding for hooks and an
API to use them requires good knowledge of where exactly you want to
hook and what API you want to provide. Then that API needs to remain
stable for an extended time. I don't think any part of the bg worker
infrastructure currently is anywhere close to that.
> With the patch, you can extend the coordinator
> protocols with IMessage, but it requires patches to core at handle_imessage().
> If you want fully-extensible workers, we should provide a method to develop
> worker codes in an external plugin.
It's originally intended as an internal infrastructure. Offering its
capabilities to the outside would requires stabilization, security
control and working out an API. All of which is certainly not something
I intend to do.
> Is it possible to develop your own codes in the plugin? If possible, you can
> use IMessage as a private protocol freely in the plugin. Am I missing something?
Well, what problem(s) are you trying to solve with such a thing? I've no
idea what direction you are aiming at, sorry. However, it's certainly
different or extending bg worker, so it would need to be a separate
patch, IMO.
Regards
Markus Wanner