Tim Armstrong has posted comments on this change. Change subject: IMPALA-3344: Simplify sorter and document/enforce invariants. ......................................................................
Patch Set 14: (2 comments) http://gerrit.cloudera.org:8080/#/c/2826/14/be/src/runtime/sorter.cc File be/src/runtime/sorter.cc: Line 71: If the run is unsorted, it is sorted in-place in : /// memory. Once sorted, runs can be spilled to disk if the sorter does not have enough : /// memory. > The part I was looking to clarify was what does "if the sorter does not hav I reworded the clause as "to free up memory". I think that wording avoids talking about the sorter, just why you might want to spill a run. Line 1456: merging_runs_.clear(); > Did you conclude anything? In general Reset() has to support this but we shouldn't have sort nodes with limits inside subplans (it should always be a TopN node) currently. So I think I'll leave the code as-is since it needs to clean up resources associated with the last batch in the merging case. I'm actually not sure if the other exec nodes handle resource transfer correctly in this case (subplan with a limit on the exec node). I filed https://issues.cloudera.org/browse/IMPALA-3651 -- 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: 14 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
