Re: assessing parallel-safety

On Wed, Mar 18, 2015 at 9:31 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Tue, Mar 17, 2015 at 9:48 AM, Robert Haas <robertmhaas(at)gmail(dot)com>
wrote:
> > On Tue, Mar 17, 2015 at 2:26 AM, Noah Misch <noah(at)leadboat(dot)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