Dmitry Lychagin has posted comments on this change. Change subject: [ASTERIXDB-2286][COMP][FUN][HYR] Parallel Sort Optimization ......................................................................
Patch Set 25: (5 comments) https://asterix-gerrit.ics.uci.edu/#/c/2393/25/asterixdb/asterix-app/src/test/resources/optimizerts/queries/hints/fullparallelsort.sqlpp File asterixdb/asterix-app/src/test/resources/optimizerts/queries/hints/fullparallelsort.sqlpp: Line 21: * Description : Test no full parallel hint to disable full parallel sort. the description is incorrect. https://asterix-gerrit.ics.uci.edu/#/c/2393/25/asterixdb/asterix-app/src/test/resources/optimizerts/results/introhashpartitionmerge.plan File asterixdb/asterix-app/src/test/resources/optimizerts/results/introhashpartitionmerge.plan: Line 16 I think we need to turn off parallel sort for this query because the intent was to test HASH_PARTITION_MERGE_EXCHANGE and it's no longer in the plan with full parallel sort https://asterix-gerrit.ics.uci.edu/#/c/2393/25/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/RangePartitionExchangePOperator.java File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/RangePartitionExchangePOperator.java: Line 136: final String splitCount = rangeMap == null ? "" : Integer.toString(rangeMap.getSplitCount()); minor. let's no print "SPLIT COUNT: " if range map is dynamic so there's no split count available at compile time. (we can do it in a follow up change) https://asterix-gerrit.ics.uci.edu/#/c/2393/25/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/ForwardOperatorDescriptor.java File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/ForwardOperatorDescriptor.java: Line 222: TaskUtil.put(rangeMapKeyInContext, rangeMapState.rangeMap, ctx); Seems like we can just call TaskUtil.put() in the RangeMapReaderActivityNodePushable.close() , so we won't need RangeMapState at all. Is that the case? (not critical, we can do in a follow up change) https://asterix-gerrit.ics.uci.edu/#/c/2393/25/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/sort/AbstractExternalSortRunMerger.java File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/sort/AbstractExternalSortRunMerger.java: Line 161: LOGGER.info("number of passes: " + numberOfPasses); minor. let's move it to the debug level and merge with the previous line. also put if (LOGGER.isDebugEnabled()) around it. -- To view, visit https://asterix-gerrit.ics.uci.edu/2393 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I73e128029a46f45e6b68c23dfb9310d5de10582f Gerrit-PatchSet: 25 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Ildar Absalyamov <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Taewoo Kim <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-HasComments: Yes
