Michael Ho has posted comments on this change.

Change subject: IMPALA-2399: Check for mem limit in allocations in parquet 
scanner and decompressor
......................................................................


Patch Set 13:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/2203/13/be/src/exec/hdfs-scanner.cc
File be/src/exec/hdfs-scanner.cc:

Line 198:     RETURN_IF_ERROR(parse_status_);
> Let's get rid of parse_status_ use in GetCollectionMemory(). parse_status_ 
Done. The new patch will revert to the previous versions' behavior of merging 
the query status into parse_status_. Also updated GetCollectionMemory().

To avoid logging unrelated query_status_ (which is a problem pointed out by 
Skye), we will check the query_status_ first before logging the parse status in 
ProcessSplit(). Please see comments there.


Line 201: GetQueryState
> Should this say GetQueryStatus()?
Oh yes. Fixed.


http://gerrit.cloudera.org:8080/#/c/2203/13/be/src/exec/hdfs-scanner.h
File be/src/exec/hdfs-scanner.h:

Line 228: scanner_inited_
> scanner_prepared_.  Or how about just calling it prepared_ since "scanner" 
Done


http://gerrit.cloudera.org:8080/#/c/2203/13/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:

Line 242: inline
> why? this path is only used when generating an error so it's not a perf cri
Removed.


-- 
To view, visit http://gerrit.cloudera.org:8080/2203
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic70400407b7662999332448f4d1bce2cc344ca89
Gerrit-PatchSet: 13
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Michael Ho <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to