Skye Wanderman-Milne has posted comments on this change.

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


Patch Set 10:

(1 comment)

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

Line 1681:       if (UNLIKELY(!parse_status_.ok())) continue_execution = false;
> Or move CheckQueryState() out of CommitRows() ?
Looking some more, the scanners actually do return a bad query status from 
ProcessSplit(). It looks the "scan node ran into an error" message will be 
printed to VLOG_QUERY, not the shell, so setting parse_status_ is arguably OK. 
Note that this will duplicate the bad query status in the runtime state error 
log, but it won't attributed it to the scan node in particular.

I still think you should set parse_status_ in CommitRows() though (or actually 
StartNewRowBatch()), that's more consistent with GetCollectionMemory(), is 
setting parse_status_ closer to where the error actually happens, and is only 
setting it when it's a problem in the scanner.

To answer your question about ProcessSplit(), there's a call to 
CheckQueryState().


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