Mailing-List: contact dev-help@drill.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@drill.apache.org
Content-Type: multipart/alternative;
 boundary="===============0871484877627965142=="
MIME-Version: 1.0
Subject: Re: Review Request 31107: Ability to make PartitionSender
 multithreaded - useful in case of LocalExchange being enabled,
 as it allows to deal with high volume of incoming data
From: "Yuliya Feldman" <yufeldman68@gmail.com>
To: "Venki Korukanti" <venki.korukanti@gmail.com>,
 "Jacques Nadeau" <jacques.drill@gmail.com>,
 "Chris Westin" <chriswestin42@gmail.com>,
 "Steven Phillips" <sphillips@maprtech.com>
Cc: "drill" <drill-dev@incubator.apache.org>,
 "Yuliya Feldman" <yufeldman68@gmail.com>
Date: Mon, 16 Mar 2015 06:09:13 -0000
Message-ID: <20150316060913.392.55527@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Yuliya Feldman" <noreply@reviews.apache.org>
References: <20150316031826.392.61432@reviews.apache.org>
In-Reply-To: <20150316031826.392.61432@reviews.apache.org>
Reply-To: "Yuliya Feldman" <yufeldman68@gmail.com>

--===============0871484877627965142==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On March 15, 2015, 8:18 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java, line 85
> > <https://reviews.apache.org/r/31107/diff/7/?file=889340#file889340line85>
> >
> >     Just to confirm, this will only be called in rare cases, right? E.g. not for every record or even every batch.

it is used only by the method you have your comment on efficiency which is used only by receivingFragmentFinished()


> On March 15, 2015, 8:18 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java, line 106
> > <https://reviews.apache.org/r/31107/diff/7/?file=889339#file889339line106>
> >
> >     I really think you should make this logic more efficient.  Why not just have a direct array of outgoing batches by index?

I am not sure what exactly you mean, but I think it is a confusion with name of the method - it should have been called getOutgoingBatch (not getOutgoingBatches) - decorator for one in PartitionerTemplate. Since now we have list of partitioners and each of them has list of outgingbatches we need to get right partitioner first.
Corresponding method in PartitionerTemplate may not be needed - since all the logic can be done inside this one. To get precise Partitioner based just on the index may not be always precise because I am trying to balance it out and so some partitioners will have an extra OutgoingBatch - like partitioner0 has batches[1,2], partitioner1 has batches[3] - if I have 2 threads/partitioners and just 3 destinations/outgoing batches.
Let's discuss if I still miss something


- Yuliya


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31107/#review76516
-----------------------------------------------------------


On March 9, 2015, 9:31 a.m., Yuliya Feldman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31107/
> -----------------------------------------------------------
> 
> (Updated March 9, 2015, 9:31 a.m.)
> 
> 
> Review request for drill, Chris Westin, Jacques Nadeau, Steven Phillips, and Venki Korukanti.
> 
> 
> Bugs: DRILL-2210
>     https://issues.apache.org/jira/browse/DRILL-2210
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> In addition to description
> 
> Fixed few classes that did not handle multithreading well
> Added/Changed some Stats behavior to allow stats merge from multiple threads, since again this class is not suitable to be used in multithreaded environment
> Introduced new decorator class to handle multi thrteading (or not)  to minimize changes to ParitionSenderRootExec class
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/compile/CodeCompiler.java 7cc350e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java 0e9da0e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/IteratorValidator.java 64cf7c5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 7af7b65 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java a23bd7a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java 5ed9c39 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 71ffd41 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/Materializer.java 961b603 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java bbfbbcb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java 0fb10ff 
>   exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java 3d3e96f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOptiqPlans.java 4aaaa78 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31107/diff/
> 
> 
> Testing
> -------
> 
> Still need to provide Unit Tests.
> 
> Functional tests are passing
> 
> Performance tests were run and look promising for some queries
> 
> 
> Thanks,
> 
> Yuliya Feldman
> 
>


--===============0871484877627965142==--