Re: bg worker: patch 1 of 6 - permanent process

Hi,

On 09/15/2010 08:54 PM, Robert Haas wrote:
> I think that the bar for committing to another in-core replication
> solution right now is probably fairly high.

I'm not trying to convince you to accept the Postgres-R patch.. at least 
not now.

<showing-off>
BTW, that'd be what I call a huge patch:

bgworkers, excluding dynshmem and imessages: 34 files changed, 2910 insertions(+), 1421 deletions(-)

from there to Postgres-R: 98 files changed, 14856 insertions(+), 230 deletions(-)
</showing-off>

> I am pretty doubtful that
> our current architecture is going to get us to the full feature set
> we'd eventually like to have - multi-master, partial replication, etc.

Would be hard to do, due to the (physical) format of WAL, yes. That's 
why Postgres-R uses its own (logical) wire format.

>   But we're not ever going to have ten replication solutions in core,
> so we need to think pretty carefully about what we accept.

That's very understandable.

> That
> conversation probably needs to start from the other end - is the
> overall architecture correct for us? - before we get down to specific
> patches.  On the other hand, I'm very interested in laying the
> groundwork for parallel query

Cool. Maybe we should take another look at bgworkers, as soon as a 
parallel querying feature gets planned?

> and I think there are probably a number
> of bits of architecture both from this project and Postgres-XC, that
> could be valuable contributions to PostgreSQL;

(...note that Postgres-R is license compatible, as opposed to the GPL'ed 
Postgres-XC project...)

> however, in neither
> case do I expect them to be accepted without significant modification.

Sure, that's understandable as well. I've published this part of the 
infrastructure to get some feedback as early as possible on that part of 
Postgres-R.

As you can certainly imagine, it's important for me that any 
modification to such a patch from Postgres-R would still be compatible 
to what I use it for in Postgres-R and not cripple any functionality 
there, because that'd probably create more work for me than not getting 
the patch accepted upstream at all.

> I'm saying it's hard to think about committing any of them because
> they aren't really independent of each other or of other parts of
> Postgres-R.

As long as you don't consider imessages and dynshmem a part of 
Postgres-R, they are independent of the rest of Postgres-R in the 
technical sense.

And for any kind of parallel querying feature, imessages and dynshmem 
might be of help as well. So I currently don't see where I could 
de-couple these patches any further.

If you have a specific requirement, please don't hesitate to ask.

> I feel like there is an antagonistic thread to this conversation, and
> some others that we've had.  I hope I'm misreading that, because it's
> not my intent to piss you off.  I'm just offering my honest feedback.
> Your mileage may vary; others may feel differently; none of it is
> personal.

That's absolutely fine. I'm thankful for your feedback.

Also note that I initially didn't even want to add the bgworker patches 
to the commit fest. I've de-coupled and published these separate from 
Postgres-R with a) the hope to get feedback (more than for the overall 
Postgres-R patch) and b) to show others that such a facility exists and 
is ready to be reused.

I didn't really expect them to get accepted to Postgres core at the 
moment. But the Postgres team normally asks for sharing concepts and 
ideas as early as possible...

> OK, I think I understand what you're trying to say now.  I guess I
> feel like the ideal architecture for any sort of solution that needs a
> pool of workers would be to keep around the workers that most recently
> proved to be useful.  Upon needing a new worker, you look for one
> that's available and already bound to the correct database.  If you
> find one, you assign him to the new task.

That's mostly how bgworkers are designed, yes. The min/max idle 
background worker GUCs allow a loose control over how many spare 
processes you want to allow hanging around doing nothing.

> If not, you find the one
> that's been idle longest and either (a) kill him off and start a new
> one that is bound to the correct database or, even better, (b) tell
> him to flush his caches and rebind to the correct database.

Hm.. sorry if I didn't express this more clearly. What I'm trying to say 
is that (b) isn't worth implementing, because it doesn't offer enough of 
an improvement over (a). The only saving would be the fork() and some 
basic process initialization.

Being able to re-use a bgworker connected to the correct database 
already gives you most of the benefit, namely not having to fork() *and* 
re-connect to the database for every job.


Back at the technical issues, let me try to summarize the feedback and 
what I do with it.

In general, there's not much use for bgworkers for just autovacuum as 
the only background job. I agree.

Tom raised the 'lots of databases' issue. I agree that the bgworker 
infrastructure isn't optimized for such a work load, but argue that it's 
configurable to not hurt. If bgworkers ever gets accepted upstream, we'd 
certainly need to discuss about reasonable defaults for the relevant 
GUCs. Additionally, more cleverness about when to start or stop (spare) 
workers from the coordinator couldn't hurt.

I had a lengthy discussion with Dimitri about whether or not bgworkers 
could help him with some kind of PgQ daemon. I think we now agree that 
bgworkers isn't the right tool for that job.

You are questioning, whether the min_idle_bgworkers GUC is really 
necessary. I'm arguing that it is necessary in Postgres-R to cover load 
spikes, because starting bgworkers is slow.


So, overall, I now got quite a bit of feedback. There doesn't seem to be 
any stumbling block in the general design of bgworkers. So I'll happily 
continue to use (and refine) bgworkers for Postgres-R. And I'm looking 
forward to more discussions once parallel querying gets more serious 
attention.

Regards

Markus Wanner