On Wed, Mar 18, 2015 at 9:31 PM, Robert Haas <robertmh...@gmail.com> wrote: > > On Tue, Mar 17, 2015 at 9:48 AM, Robert Haas <robertmh...@gmail.com> wrote: > > On Tue, Mar 17, 2015 at 2:26 AM, Noah Misch <n...@leadboat.com> wrote: > >> Neither that rule, nor its variant downthread, would hurt operator authors too > >> much. To make the planner categorically parallel-safe, though, means limiting > >> evaluate_function() to parallel-safe functions. That would dramatically slow > >> selected queries. It's enough for the PL scenario if planning a parallel-safe > >> query is itself parallel-safe. If the planner is parallel-unsafe when > >> planning a parallel-unsafe query, what would suffer? > > > > Good point. So I guess the rule can be that planning a parallel-safe > > query should be parallel-safe. From there, it follows that estimators > > for a parallel-safe operator must also be parallel-safe. Which seems > > fine. > > More work is needed here, but for now, here is a rebased patch, per > Amit's request. >
Thanks for rebasing the patch. I have done some performance testing of this + parallel-mode-v8.1.patch to see the impact of these patches for non-parallel statements. First set of tests are done with pgbench read-only workload Test Environment hydra: (IBM POWER-7 16 cores, 64 hardware threads) Below data is median of 3 runs (detailed data of 3 runs can be found in attached document pert_data_assess_parallel_safety.ods): Shared_buffers- 8GB Scale Factor - 100 HEAD - Commit - 8d1f2390 *1* *8* *16* *32* HEAD 9098 70080 129891 195862 PATCH 8960 69678 130591 195570 This data shows there is no performance impact (apart from 1~2% run-to-run difference, which I consider as noise) of these patches on read only workload. Second set of test constitutes of long sql statements with many expressions in where clause to check the impact of extra-pass over query tree in planner to decide if it contains any parallel-unsafe function. Before executing below tests, execute test_prepare_parallel_safety.sql script Test-1 ----------- SQL statement containing 100 expressions (expressions are such that they have CoerceViaIO node (presumably the costliest one in function contain_parallel_unsafe_walker())) in where clause, refer attached sql script (test_parallel_safety.sql) Data for 10 runs (Highest of 10 runs): HEAD - 1.563 ms PATCH - 1.589 ms Test-2 ------------ Similar SQL statement as used for Test-1, but containing 500 expressions, refer attached script (test_more_parallel_safety.sql) Data for 5 runs (Median of 5 runs): HEAD - 5.011 ms PATCH - 5.201 ms Second set of tests shows that there is about 1.5 to 3.8% performance regression with patches. I think having such long expressions and that to containing CoerceViaIO nodes is very unusual scenario, so this doesn't seem to be too much of a concern. Apart from this, I have one observation: static int exec_stmt_execsql(PLpgSQL_execstate *estate, PLpgSQL_stmt_execsql *stmt) { ParamListInfo paramLI; long tcount; int rc; PLpgSQL_expr *expr = stmt->sqlstmt; /* * On the first call for this statement generate the plan, and detect * whether the statement is INSERT/UPDATE/DELETE */ if (expr->plan == NULL) { ListCell *l; exec_prepare_plan(estate, expr, 0); Shouldn't we need parallelOk in function exec_stmt_execsql() to pass cursoroption in above function as we have done in exec_run_select()? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
perf_data_assess_parallel_safety.ods
Description: application/vnd.oasis.opendocument.spreadsheet
test_prepare_parallel_safety.sql
Description: Binary data
test_parallel_safety.sql
Description: Binary data
test_more_parallel_safety.sql
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