Tim Armstrong has posted comments on this change. Change subject: IMPALA-3344: Simplify sorter and document/enforce invariants. ......................................................................
Patch Set 16: (9 comments) http://gerrit.cloudera.org:8080/#/c/2826/16/be/src/runtime/sorter.cc File be/src/runtime/sorter.cc: Line 102: in a batch > from 'batch' Done Line 159: runs > 'runs' Done Line 159: vector > 'runs' (not necessarily a vector). Done Line 720: var_len_copy_block_ = NULL; > okay to ignore, but maybe move this to line 673, and then the DCHECK at L68 Done Line 1055: tuple_ > is this referring to the member (tuple_) or should it just say tuple? It was a typo Line 1458: > remove extraneous blank line Done http://gerrit.cloudera.org:8080/#/c/2826/16/be/src/runtime/sorter.h File be/src/runtime/sorter.h: Line 52: > a Done http://gerrit.cloudera.org:8080/#/c/2826/16/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test File testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test: Line 1588: SET max_block_mgr_memory=36m; > why did this change? see below. Line 1596: SET max_block_mgr_memory=64m; > and this? did something change in your last patch to necessitate this? I tuned it locally, but the first time around I accidentally had the mini-cluster leftover from test_spilling, which has a tiny block size. Meant to leave a comment but forgot. -- To view, visit http://gerrit.cloudera.org:8080/2826 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9c619e81fd1b8ac50e257172c8bce101a112b52a Gerrit-PatchSet: 16 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
