On Tue, Feb 23, 2016 at 6:45 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Tue, Feb 23, 2016 at 2:06 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>>> Foreign tables are supposed to be categorically excluded from
>>>> parallelism. Not sure why that's not working in this instance.
>>
>> BTW, I wonder where you think that's supposed to be enforced, because
>> I sure can't find any such logic.
>>
>> I suppose that has_parallel_hazard() would be the logical place to
>> notice foreign tables, but it currently doesn't even visit RTEs,
>> much less contain any code to check if their tables are foreign.
>> Or did you have another place in mind to do that?
>
> RTEs are checked in set_rel_consider_parallel(), and I thought there
> was a check there related to foreign tables, but there isn't. Oops.
> In view of 69d34408e5e7adcef8ef2f4e9c4f2919637e9a06, we shouldn't
> blindly assume that foreign scans are not parallel-safe, but we can't
> blindly assume the opposite either. Maybe we should assume that the
> foreign scan is parallel-safe only if one or more of the new methods
> introduced by the aforementioned commit are set, but actually that
> doesn't seem quite right. That would tell us whether the scan itself
> can be parallelized, not whether it's safe to run serially but within
> a parallel worker. I think maybe we need a new FDW API that gets
> called from set_rel_consider_parallel() with the root, rel, and rte as
> arguments and which can return a Boolean. If the callback is not set,
> assume false.
Here is a first pass at that. The patch adds
IsForeignScanParallelSafe to the FDW API. postgres_fdw returns false
(unnecessary but useful to verify that the regression test breaks if
you change it to true), and others don't provide the function so fall
back to false.
I suspect there may be opportunities to return true even if snapshots
and uncommitted reads aren't magically coordinated among workers. For
example: users of MongoDB-type systems and text files have no
expectation of either snapshot semantics or transaction isolation in
the first place, so doing stuff in parallel won't be any less safe
than usual as far as visibility is concerned; postgres_fdw could in
theory export/import snapshots and allow parallelism in limited cases
if it can somehow prove there have been no uncommitted writes; and
non-MVCC/snapshot RDBMSs might be OK in lower isolation levels if you
haven't written anything or have explicitly opted in to uncommitted
reads (otherwise you'd risk invisible deadlock against the leader when
trying to read what it has written).
Please also find attached a tiny patch to respect TEMP_CONFIG for contribs.
--
Thomas Munro
http://www.enterprisedb.com