On Tue, Feb 23, 2016 at 6:45 PM, Robert Haas <robertmh...@gmail.com> wrote: > On Tue, Feb 23, 2016 at 2:06 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: >> Robert Haas <robertmh...@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
temp-config-for-contribs.patch
Description: Binary data
fdw-parallel-safe-api.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers