Dan Hecht 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.
> I think that's starting to describe the algorithm implemented by the sorter
The part I was looking to clarify was what does "if the sorter does not have 
enough memory" mean.  I see your point, so maybe just delete that clause since 
the decision when to spill is up to the sorter.


Line 1456:   merging_runs_.clear();
> This invariants around Reset() aren't clear. I added logic here to clean up
Did you conclude anything?


-- 
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

Reply via email to