On Thu, Sep 10, 2015 at 2:12 PM, Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Thu, Sep 10, 2015 at 4:16 AM, Robert Haas <robertmh...@gmail.com> wrote:
>>
>> On Wed, Sep 9, 2015 at 11:07 AM, Amit Kapila <amit.kapil...@gmail.com>
>> wrote:
>> > On Wed, Sep 9, 2015 at 11:47 AM, Haribabu Kommi
>> > <kommi.harib...@gmail.com>
>> > wrote:
>> >> With subquery, parallel scan is having some problem, please refer
>> >> below.
>> >>
>> >> postgres=# explain analyze select * from test01 where kinkocord not in
>> >> (select kinkocord from test02 where tenpocord = '001');
>> >> ERROR:  badly formatted node string "SUBPLAN :subLinkType 2
>> >> :testexpr"...
>> >> CONTEXT:  parallel worker, pid 32879
>> >> postgres=#
>> >
>> > The problem here is that readfuncs.c doesn't have support for reading
>> > SubPlan nodes. I have added support for some of the nodes, but it seems
>> > SubPlan node also needs to be added.  Now I think this is okay if the
>> > SubPlan
>> > is any node other than Funnel, but if Subplan contains Funnel, then each
>> > worker needs to spawn other workers to execute the Subplan which I am
>> > not sure is the best way.  Another possibility could be store the
>> > results of
>> > Subplan in some tuplestore or some other way and then pass those to
>> > workers
>> > which again doesn't sound to be promising way considering we might have
>> > hashed SubPlan for which we need to build a hashtable.  Yet another way
>> > could be for such cases execute the Filter in master node only.
>>
>> IIUC, there are two separate issues here:
>>
>
> Yes.
>
>> 1. We need to have readfuncs support for all the right plan nodes.
>> Maybe we should just bite the bullet and add readfuncs support for all
>> plan nodes.  But if not, we can add support for whatever we need.
>>
>> 2. I think it's probably a good idea - at least for now, and maybe
>> forever - to avoid nesting parallel plans inside of other parallel
>> plans.  It's hard to imagine that being a win in a case like this, and
>> it certainly adds a lot more cases to think about.
>>
>
> I also think that avoiding nested parallel plans is a good step forward.
>

I reviewed the parallel_seqscan_funnel_v17.patch and following are my comments.
I will continue my review with the parallel_seqscan_partialseqscan_v17.patch.

+ if (inst_options)
+ {
+ instoptions = shm_toc_lookup(toc, PARALLEL_KEY_INST_OPTIONS);
+ *inst_options = *instoptions;
+ if (inst_options)

Same pointer variable check, it should be if (*inst_options) as per the
estimate and store functions.


+ if (funnelstate->ss.ps.ps_ProjInfo)
+ slot = funnelstate->ss.ps.ps_ProjInfo->pi_slot;
+ else
+ slot = funnelstate->ss.ss_ScanTupleSlot;

Currently, there will not be a projinfo for funnel node. So always it uses
the scan tuple slot. In case if it is different, we need to add the ExecProject
call in ExecFunnel function. Currently it is not present, either we can document
it or add the function call.


+ if (!((*dest->receiveSlot) (slot, dest)))
+ break;

and

+void
+TupleQueueFunnelShutdown(TupleQueueFunnel *funnel)
+{
+ if (funnel)
+ {
+ int i;
+ shm_mq_handle *mqh;
+ shm_mq   *mq;
+ for (i = 0; i < funnel->nqueues; i++)
+ {
+ mqh = funnel->queue[i];
+ mq = shm_mq_get_queue(mqh);
+ shm_mq_detach(mq);
+ }
+ }
+}


Using this function, the backend detaches from the message queue, so
that the workers
which are trying to put results into the queues gets an error message
as SHM_MQ_DETACHED.
Then worker finshes the execution of the plan. For this reason all the
printtup return
types are changed from void to bool.

But this way the worker doesn't get exited until it tries to put a
tuple in the queue.
If there are no valid tuples that satisfy the condition, then it may
take time for the workers
to exit. Am I correct? I am not sure how frequent such scenarios can occur.


+ if (parallel_seqscan_degree >= MaxConnections)
+ {
+ write_stderr("%s: parallel_scan_degree must be less than
max_connections\n", progname);
+ ExitPostmaster(1);
+ }

The error condition works only during server start. User still can set
parallel seqscan degree
more than max connection at super user session level and etc.


+ if (!parallelstmt->inst_options)
+ (*receiver->rDestroy) (receiver);

Why only when there is no instruementation only, the receiver needs to
be destroyed?


Regards,
Hari Babu
Fujitsu Australia


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to