Re: parallel mode and parallel contexts

On 6 January 2015 at 16:33, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

>> These comments don’t have any explanation or justification
>
> OK, I rewrote them. Hopefully it's better now.

Thanks for new version and answers.

>> * Transaction stuff strikes me as overcomplicated and error prone.
>> Given there is no explanation or justification for most of it, I’d be
>> inclined to question why its there
>
> Gosh, I was pretty pleased with how simple the transaction integration
> turned out to be. Most of what's there right now is either (a) code
> to copy state from the master to the parallel workers or (b) code to
> throw errors if the workers try to things that aren't safe. I suspect
> there are a few things missing, but I don't see anything there that
> looks unnecessary.

If you can explain it in more detail in comments and README, I may
agree. At present, I don't get it and it makes me nervous.

The comment says
"Only the top frame of the transaction state stack is copied to a parallel
worker"
but I'm not sure why.

Top meaning the current subxact or the main xact?
If main, why are do we need XactTopTransactionId

>> This is very impressive, but it looks like we’re trying to lift too
>> much weight on the first lift. If we want to go anywhere near this, we
>> need to have very clear documentation about how and why its like that.
>> I’m actually very sorry to say that because the review started very
>> well, much much better than most.
>
> When I posted the group locking patch, it got criticized because it
> didn't actually do anything useful by itself; similarly, the
> pg_background patch was criticized for not being a large enough step
> toward parallelism. So, this time, I posted something more
> comprehensive. I don't think it's quite complete yet. I expect a
> committable version of this patch to be maybe another 500-1000 lines
> over what I have here right now -- I think it needs to do something
> about heavyweight locking, and I expect that there are some unsafe
> things that aren't quite prohibited yet. But the current patch is
> only 2300 lines, which is not astonishingly large for a feature of
> this magnitude; if anything, I'd say it's surprisingly small, due to a
> year and a half of effort laying the necessary groundwork via long
> series of preliminary commits. I'm not unwilling to divide this up
> some more if we can agree on a way to do that that makes sense, but I
> think we're nearing the point where we need to take the plunge and
> say, look, this is version one of parallelism. Thunk.

I want this also; the only debate is where to draw the line and please
don't see that as criticism.

I'm very happy it's so short, I agree it could be longer.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services