Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-3823: Add timer to measure Parquet footer reads
......................................................................


Patch Set 5:

(4 comments)

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

Line 228:       process_footer_avg_timer_(NULL),
> there seem to be a bunch more uninitialized counters, could you also initia
Done


Line 1887:   {
> this isn't quite everything we want, because it doesn't include the hdfs re
As we spoke, the read inside the ProcessFooter() is blocking and so it will 
capture that time as well.


http://gerrit.cloudera.org:8080/#/c/3576/3/be/src/exec/hdfs-parquet-scanner.h
File be/src/exec/hdfs-parquet-scanner.h:

Line 471:   /// Average time spent processing the footer by each split.
> what do you mean by "helper timer"?
I did it a different way in the first patch and then fixed it in the following 
patches. So I had that comment removed in the following patch.


http://gerrit.cloudera.org:8080/#/c/3576/3/be/src/util/runtime-profile.h
File be/src/util/runtime-profile.h:

PS3, Line 154: .
> "profile with"
I had this comment corrected in the following patch too.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf87bad90037dd0cea63b10c537382ec0f980cbf
Gerrit-PatchSet: 5
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-HasComments: Yes

Reply via email to