Re: assessing parallel-safety

On Fri, Mar 20, 2015 at 7:54 PM, Thom Brown <thom(at)linux(dot)com> wrote:
>
> On 20 March 2015 at 13:55, Thom Brown <thom(at)linux(dot)com> wrote:
> > On 20 March 2015 at 13:16, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
wrote:
> >> Thom Brown wrote:
> >>> On 18 March 2015 at 16:01, 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.
> >>>
> >>> This no longer applies due to changes in commit
> >>> 13dbc7a824b3f905904cab51840d37f31a07a9ef.
> >>
> >> You should be able to drop the pg_proc.h changes and run the supplied
> >> perl program. (I'm not sure that sending the patched pg_proc.h
together
> >> with this patch is all that useful, really.)
> >
> > Thanks. All patches applied and building okay.
>
> Okay, breakage experienced, but not sure which thread this belongs on.
>

I think if you face the issue issue after applying parallel_seqscan patch,
then you can report on that thread and if it turns out to be something
related to other thread, then we can shift the discussion of resolution
on that thread.

> createdb pgbench
> pgbench -i -s 200 pgbench
>
> CREATE TABLE pgbench_accounts_1 (CHECK (bid = 1)) INHERITS
(pgbench_accounts);
> ...
> CREATE TABLE pgbench_accounts_200 (CHECK (bid = 200)) INHERITS
> (pgbench_accounts);
>

I managed to reproduce the Assertion reported by you as:

#2 0x00000000007a053a in ExceptionalCondition
(conditionName=conditionName(at)entry=0x813a4b
"!(IsInParallelMode())", errorType=errorType(at)entry=0x7da1d6
"FailedAssertion", fileName=fileName(at)entry=0x81397d "parallel.c",
lineNumber=lineNumber(at)entry=123) at assert.c:54
#3 0x00000000004cd5ba in CreateParallelContext (entrypoint=entrypoint(at)entry
=0x659d2c <ParallelQueryMain>, nworkers=nworkers(at)entry=8) at parallel.c:123

The reason is that CreateParallelContext() expects to be called
in ParallelMode and we enter into parallel-mode after InitPlan()
in standard_ExecutorStart(). So the probable fix could be
to EnterParallelMode before initializing the plan.

I still could not reproduce the crash you have reported as:
>> #0 0x0000000000770843 in pfree ()
>> #1 0x00000000005a382f in ExecEndFunnel ()
>> #2 0x000000000059fe75 in ExecEndAppend ()
>> #3 0x00000000005920bd in standard_ExecutorEnd ()

Could you let me know which all patches you have tried
and on top of which commit.

I am trying on the commit as mentioned in mail[1]. Basically
have you tried the versions mentioned in that mail:

HEAD Commit-id : 8d1f2390
parallel-mode-v8.1.patch [2]
assess-parallel-safety-v4.patch [1]
parallel-heap-scan.patch [3]
parallel_seqscan_v11.patch (Attached with this mail)

If something else, could you let me know the same so that I can try
that to reproduce the issue reported by you.

[1]
http://www.postgresql.org/message-id/CAA4eK1JSSonzKSN=L-DWuCEWdLqkbMUjvfpE3fGW2tn2zPo2RQ@mail.gmail.com

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com