Re: auto_explain vs. parallel query

On 11/03/2016 03:59 PM, Robert Haas wrote:
> On Wed, Nov 2, 2016 at 12:49 PM, Tomas Vondra
> <tomas.vondra@2ndquadrant.com> wrote:
>> On 11/01/2016 08:32 PM, Robert Haas wrote:
>>> On Tue, Nov 1, 2016 at 10:58 AM, Tomas Vondra
>>> <tomas.vondra@2ndquadrant.com> wrote:
>>>>
>>>> Damn! You're right of course. Who'd guess I need more coffee this early?
>>>>
>>>> Attached is a fix replacing the flag with an array of flags, indexed by
>>>> ParallelMasterBackendId. Hopefully that makes it work with multiple
>>>> concurrent parallel queries ... still, I'm not sure this is the right
>>>> solution.
>>>
>>> I feel like it isn't.  I feel like this ought to go in the DSM for
>>> that parallel query, not the main shared memory segment, but I'm not
>>> sure how to accomplish that offhand.  Also, if we do solve it this
>>> way, surely we don't need the locking.  The flag's only set before any
>>> workers have started and never changes thereafter.
>>
>> I'm not sure what you mean by "DSM for that parallel query" - I thought the
>> segments are created for Gather nodes, no? Or is there a DSM for the whole
>> query that we could use?
>
> Sure, the Gather node creates it. There's generally only one per
> query, though, and that's how most information is communicated from
> leader to workers.
>

Ah, right. I haven't realized there's just a single Gather per query.
>>
>> Another thing is that maybe we don't really want to give extensions access
>> to any of those segments - my impression was those segments are considered
>> internal (is there RequestAddinShmemSpace for them?). And hacking something
>> just for auto_explain seems a big ugly.
>
> Yeah.  I thought that there wouldn't be any reason for third-party
> code to need to get into these segments, but maybe that was
> short-sighted of me.  If we fix this without that, then we've got to
> force pg_stat_statements to be loaded through
> shared_preload_libarries, as you mentioned, and that doesn't seem nice
> either.

Well, I was talking about auto_explain, and pg_stat_statements already 
has to be loaded through shared_preload_libraries. I'm not sure how many 
other extensions would need similar hack (I don't see any in contrib, 
but there may be external ones).

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services