Re: parallel mode and parallel contexts

On Fri, Jan 16, 2015 at 8:05 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> I'm not sure either. But I think the current location is wrong anyway -
> during AtEOXact_Parallel() before running user defined queries via
> AfterTriggerFireDeferred() seems wrong.

Good point.

>> I'm fine with adding checks to heap_insert() and
>> heap_inplace_update().  I'm not sure we really need to add anything to
>> index_insert(); how are we going to get there without hitting some
>> other prohibited operation first?
>
> I'm not sure. But it's not that hard to imagine that somebody will start
> adding codepaths that insert into indexes first. Think upsert.

OK, but what's the specific reason it's unsafe?  The motivation for
prohibiting update and delete is that, if a new combo CID were to be
created mid-scan, we have no way to make other workers aware of it.
There's no special reason to think that heap_insert() or
heap_inplace_update() are unsafe, but, sure, we can prohibit them for
symmetry.  If we're going to start extending the net further and
further, we should have specific comments explaining what the hazards.
People will eventually want to relax these restrictions, I think, and
there's nothing scarier than removing a prohibition that has
absolutely no comments to explain why that thing was restricted in the
first place.

> As far as I can see this could relatively easily be implemented ontop of
> the removal of ImmediateInterruptOK in the patch series I posted
> yesterday? Afaics you just need to remove most of
> +HandleParallelMessageInterrupt() and replace it by a single SetLatch().

That would be swell.  I'll have a look, when I have time, or when it's
committed, whichever happens first.

>> > * I'm not a fan of the shm_toc stuff... Verbose and hard to read. I'd
>> >   much rather place a struct there and be careful about not using
>> >   pointers. That also would obliviate the need to reserve some ids.
>>
>> I don't see how that's going to work with variable-size data
>> structures, and a bunch of the things that we need to serialize are
>> variable-size.
>
> Meh. Just appending the variable data to the end of the structure and
> calculating offsets works just fine.

I think coding it all up ad-hoc would be pretty bug-prone.  This
provides some structure, where each module only needs to know about
its own serialization format.  To me, that's a lot easier to work
with.

New patch attached.  I'm going to take the risk of calling this v1
(previous versions have been 0.x), since I've now done something about
the heavyweight locking issue, as well as fixed the message-looping
bug Amit pointed out.  It doubtless needs more work, but it's starting
to smell a bit more like a real patch.

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