> On Feb. 5, 2015, 11:04 p.m., Jacques Nadeau wrote: > > PojoRecordReader needs infinite affinity. Please fix and add test case > > with slice target as 1 and group by/sort to force exchange insertion and > > make sure that infinite affinity works correctly (maybe add a few more > > cases on just that sub functionality) > > > > Why don’t we always have small affinity to previous node for other > > exchanges?
TestLocalExchange.java already contains tests for groupby/join on tables that have multiple input files and slice_target set to 1. In the new patch removed NO_AFFINITY, added exchange default affinity as RECEIVER_AFFINITY_TO_SENDER and receiver affinities to sender location. > On Feb. 5, 2015, 11:04 p.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/EndpointAffinity.java, > > line 68 > > <https://reviews.apache.org/r/30466/diff/3/?file=848072#file848072line68> > > > > This seems misnamed. Maybe isAffinityRequired() Renamed to isAssignmentRequired(). > On Feb. 5, 2015, 11:04 p.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/Receiver.java, > > line 38 > > <https://reviews.apache.org/r/30466/diff/3/?file=848081#file848081line38> > > > > If we are using this all over, let's use > > IntObjectOpenHashMap<DrillbitEndpoint> Move to List<MinorFragmentEndpoint> where MinorFragmentEndpoint object contains fragment id and Drillbit endpoint. > On Feb. 5, 2015, 11:04 p.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/Wrapper.java, > > line 235 > > <https://reviews.apache.org/r/30466/diff/3/?file=848112#file848112line235> > > > > This naming is a bit misleading. It sounds like whether or not this > > will be parallel but I believe it is actually have we already made > > parallelization decisions about this. If the latter, we should give a more > > descriptive name. Removed to isEndpointAssignmentDone() > On Feb. 5, 2015, 11:04 p.m., Jacques Nadeau wrote: > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestLocalExchange.java, > > line 297 > > <https://reviews.apache.org/r/30466/diff/3/?file=848130#file848130line297> > > > > can we move to common? Removed this as Guava Files provides APIs for creating temp dir/files in process temp directory. Using those APIs. > On Feb. 5, 2015, 11:04 p.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/GroupScan.java, > > line 60 > > <https://reviews.apache.org/r/30466/diff/3/?file=848078#file848078line60> > > > > It seems ParallelizationInfo and these other methods are redundant. > > Why do we have both? > > > > Also wondering why you deleted HasAffinity. The idea was that any node > > could have affinity and that we ultimately choose to understand affinity > > based on that. Reverted the HasAffinity interface and now GroupScan and Store continue to use that interface. - Venki ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30466/#review70841 ----------------------------------------------------------- On Feb. 4, 2015, 4:30 p.m., Venki Korukanti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30466/ > ----------------------------------------------------------- > > (Updated Feb. 4, 2015, 4:30 p.m.) > > > Review request for drill, Chris Westin, Jacques Nadeau, and Steven Phillips. > > > Repository: drill-git > > > Description > ------- > > In this patch LocalExchange contains only multiplexing exchange. Currently > working on demultiplexing exchange (there are few failures in executions and > currently debugging those). Sending this patch to get initial feedback. > > Brief overview of changes: > 1. Traverse the PRel tree after all optimizations. Whenever a > HashToRandomExchangePrel is encountered insert a MuxExchange before > HashToRandomExchangePrel and DemuxExchange after HashToRandomExchangePrel. > 2. Parallelization changes: > i) Traverse the physical operator tree and divide it into Fragments. > ii) Based on the affinity of the sending Exchange, set parallelization > dependencies between fragments. > iii) Start parallelizing from the leaf fragments (fragment that have no > other fragments depending on them for parallelization info). Stats collection > include collecting parallelization info (minWidth, maxWidth, affinityMap) and > cost. > 3. Change the Receiver to accept set of (minorFragmentId, DrillbitEndpoint) > as sender list. This also involved few changes in DataCollector. > 4. Change SingleSender to accept custom minorFragmentId instead of default > minorFragmentId of 0 > > > Diffs > ----- > > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/EndpointAffinity.java > df31f74 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractExchange.java > 73280ea > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractGroupScan.java > 5d0d9bf > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractReceiver.java > f621a26 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractSender.java > 53a0721 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/Exchange.java > 7be7f20 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/GroupScan.java > 23860a3 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/HasAffinity.java > 52462db > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/PhysicalOperatorUtil.java > dfcb113 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/Receiver.java > 0c67770 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/Sender.java > bbd1b2c > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/Store.java > 94411ea > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/BroadcastExchange.java > 73a1d20 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/BroadcastSender.java > 1827367 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/DeMuxExchange.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/HashPartitionSender.java > bdb1362 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/HashToMergeExchange.java > f62d922 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/HashToRandomExchange.java > fac374b > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/MergingReceiverPOP.java > f5dca1a > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/MuxExchange.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/OrderedPartitionExchange.java > 8e1526a > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/OrderedPartitionSender.java > 0a2b9be > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/RangeSender.java > c8c8f43 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/Screen.java > 58c8e29 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/SingleMergeExchange.java > 2914112 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/SingleSender.java > 4a11a51 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/UnionExchange.java > cfc21ac > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/UnorderedReceiver.java > 3a4dd0e > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java > 6db9f4a > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java > 22fa047 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java > f09acaa > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java > 4292c09 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/Fragment.java > ac63bde > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/MakeFragmentsVisitor.java > 8756e5b > > 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/fragment/ParallelizationInfo.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/PlanningSet.java > 8cc6c85 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/SimpleParallelizer.java > 434cdd4 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/Stats.java > eda364b > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/StatsCollector.java > 41ff678 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/Wrapper.java > 86b395e > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/DeMuxExchangePrel.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/MuxExchangePrel.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java > faa8546 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/InsertLocalExchangeVisitor.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java > 79603eb > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java > f20627d > > exec/java-exec/src/main/java/org/apache/drill/exec/store/direct/DirectGroupScan.java > aa1609d > > exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaGroupScan.java > 8335ed9 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/mock/MockGroupScanPOP.java > 5736df8 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/mock/MockStorePOP.java > 4c12d57 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableScan.java > 053f5de > > exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/AbstractDataCollector.java > c83106c > > exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/DataCollector.java > dc016be > > exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/IncomingBuffers.java > b0206f7 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/MergingCollector.java > ce14260 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/PartitionedCollector.java > 5190d84 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java > b33042b > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestLocalExchange.java > PRE-CREATION > exec/java-exec/src/test/java/org/apache/drill/exec/pop/PopUnitTestBase.java > 9a32ff9 > > exec/java-exec/src/test/java/org/apache/drill/exec/pop/TestFragmentChecker.java > 6349b76 > > Diff: https://reviews.apache.org/r/30466/diff/ > > > Testing > ------- > > Ran Functional and TPCH SF100 parquet verification tests. > > > Thanks, > > Venki Korukanti > >
