Re: parallel mode and parallel contexts

On Tue, Mar 24, 2015 at 3:26 PM, Andres Freund <andres@2ndquadrant.com> wrote:
>> >> +  - The currently active user ID and security context.  Note that this is
>> >> +    the fourth user ID we restore: the initial step of binding to the correct
>> >> +    database also involves restoring the authenticated user ID.  When GUC
>> >> +    values are restored, this incidentally sets SessionUserId and OuterUserId
>> >> +    to the correct values.  This final step restores CurrentUserId.
>> >
>> > Ah. That's the answer for above. Could you just move it next to the
>> > other user bit?
>>
>> Well, I think it's good to keep this in the same order it happens.
>> That's almost true, with the exception of the libraries, which were
>> out of order.  (I've fixed that now.)
>
> I don't find this convincing in the least. This should explain a
> developer which state he can rely on being shared. For that a sane order
> is helpful. In which precise order this happens doesn't seem to matter
> for that at all; it's also likely ot get out of date.

I'm hoping we can agree to disagree on this point.  I like my order
order better, you like your order better; we're arguing about the
ordering of paragraphs in a README.

>> You'd need some kind of
>> API that says "pretend I'm waiting for this lock, but don't really
>> wait for it", and you'd need to be darn sure that you removed yourself
>> from the wait queue again before doing any other heavyweight lock
>> manipulation.  Do you have specific thoughts on how to implement this?
>
> I've thought some about this, and I think it's a bit easier to not do it
> on the actual lock waitqueues, but teach deadlock.c about that kind of
> blocking.
>
> deadlock.c is far from simple, and at least I don't find the control
> flow to be particularly clear. So it's not easy.  It'd be advantageous
> to tackle things at that level because it'd avoid the need to acquire
> locks on some lock's waitqueue when blocking; we're going to do that a
> lot.
>
> But It seems to me that it should be possible to suceed: In
> FindLockCycleRecurse(), in the case that we're not waiting for an actual
> lock (checkProc->links.next == NULL) we can add a case that considers
> the 'blocking parallelism' case. ISTM that that's just a
> FindLockCycleRecurse() call on the process that we're waiting for. We'd
> either have to find the other side's locktag for DEADLOCK_INFO or invent
> another field in there; but that seems like a solveable problem.

I'll take a look at this.  Thanks for the pointers.

> That's a fair point. I was worried that this is going to introduce
> additional hard edges between processes. I think it actually might, but
> only when  a lock is upgraded; which really shouldn't happen for the
> copied locks.

Agreed.  And if it does, and we get a deadlock, I think I'm sorta OK
with that.  It's a known fact that lock upgrades are a deadlock
hazard, and the possible existence of obscure scenarios where that's
more likely when parallelism is involved than without it doesn't
bother me terribly.  I mean, we have to look at the specifics, but I
think it's far from obvious that

> I'm not sure if I said that somewhere before: If we aborted parallelism
> if any of the to-be-copied locks would conflict with its copy, instead
> of duplicating them, I could live with this. Then this would amount to
> jumping the lock queue, which seems reasonable to me.  Since not being
> able to do parallelism already needs to be handled gracefull, this
> doesn't seem to be too bad.

It's a tempting offer since it would shorten the path to getting this
committed, but I think that's leaving a lot on the table.  In
particular, if we ever want to have parallel CLUSTER, VACUUM FULL, or
ALTER TABLE, that's not going to get us there.  If we don't have the
right infrastructure to support that in whatever the initial commit
is, fine.  But we've got to have a plan that gets us there, or we're
just boxing ourselves into a corner.

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