Hello Michael Ho, Alex Behm, Dan Hecht, I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/3873 to look at the new patch set (#22). Change subject: IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder ...................................................................... IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder The main outcome of this patch is to split out PhjBuilder from PartitionedHashJoinNode, which manages the build partitions for the join and implements the DataSink interface. A lot of this work is fairly mechanical refactoring: dividing the state between the two classes and duplicating it where appropriate. One major change is that probe partitions need to be separated from build partitions. This required some significant algorithmic changes to memory management for probe partition buffers: memory management for probe partitions was entangled with the build partitions: small buffers were allocated for the probe partitions even for non-spilling joins and the join could spill additional partitions during probing if the probe partitions needed to switch from small to I/O buffers. The changes made were: - Probe partitions are only initialized after the build is partitioned, and only for spilled build partitions. - Probe partitions never use small buffers: once the initial write buffer is allocated, appending to the probe partition never fails. - All probe partition allocation is done after partitioning the build and before processing the probe input during the same phase as hash table building. (Aside from NAAJ partitions which are allocated upfront). The probe partition changes necessitated a change in BufferedTupleStream: allocation of write blocks is now explicit via the PrepareForWrite() API. Testing: Ran exhaustive build and local stress test. Memory Usage: Ran stress test binary search locally for TPC-DS SF-1 and TPC-H SF-20. No regressions on TPC-DS. TPC-H either stayed the same or improved in min memory requirement without spilling, but the min memory requirement with spilling regressed in some cases. I investigated each of the significant regressions on TPC-H and determined that they were all due to exec nodes racing for spillable or non-spillable memory. None of them were cases where exec nodes got their minimum reservation and failed to execute the spilling algorithm correctly. Change-Id: I1e02ea9c7a7d1a0f373b11aa06c3237e1c7bd4cb --- M .clang-format M be/src/codegen/gen_ir_descriptions.py M be/src/codegen/impala-ir.cc M be/src/exec/CMakeLists.txt M be/src/exec/analytic-eval-node.cc M be/src/exec/blocking-join-node.cc M be/src/exec/blocking-join-node.h M be/src/exec/data-sink.cc M be/src/exec/hash-join-node.cc M be/src/exec/nested-loop-join-builder.cc M be/src/exec/nested-loop-join-builder.h M be/src/exec/nested-loop-join-node.cc M be/src/exec/partitioned-aggregation-node.cc A be/src/exec/partitioned-hash-join-builder-ir.cc A be/src/exec/partitioned-hash-join-builder.cc A be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node-ir.cc M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h M be/src/exec/partitioned-hash-join-node.inline.h M be/src/runtime/buffered-block-mgr.cc M be/src/runtime/buffered-tuple-stream-test.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/buffered-tuple-stream.h M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M tests/stress/concurrent_select.py 27 files changed, 2,264 insertions(+), 1,490 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/3873/22 -- To view, visit http://gerrit.cloudera.org:8080/3873 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1e02ea9c7a7d1a0f373b11aa06c3237e1c7bd4cb Gerrit-PatchSet: 22 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>