impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
Date Thu, 06 Oct 2016 20:36:59 GMT
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with
Kudu scan node
......................................................................


Patch Set 1:

(4 comments)

Thanks for looking at this.

I'll add someone from Kudu.

http://gerrit.cloudera.org:8080/#/c/4639/1//COMMIT_MSG
Commit Message:

PS1, Line 7: IMPALA-3920: TotalStorageWaitTime counter not populated for fragments
           : with Kudu scan node
1line. e.g. something like

IMPALA-3920: Update TotalStorageWaitTime for Kudu scans


http://gerrit.cloudera.org:8080/#/c/4639/1/be/src/exec/kudu-scanner.cc
File be/src/exec/kudu-scanner.cc:

PS1, Line 137: KUDU_RETURN_IF_ERROR(kudu::client::KuduScanToken::DeserializeIntoScanner(
             :       scan_node_->kudu_client(), scan_token, &scanner),
@Kudu-team: Is this worth accounting for?


PS1, Line 159: scanner_->Close();
@Kudu-team: Is this worth accounting for?


PS1, Line 339:  SCOPED_TIMER(scan_node_->kudu_read_timer());
It looks like this pretty much duplicates what you're doing. I think we should probably remove
this timer (everywhere) and instead update total_storage_wait_timer right here.

It looks like kudu_read_timer is also updated when calling Open() though. I'm not sure if
that's useful. Let's ask the Kudu folks. If they think we should keep timing Open() (and the
other things I pointed out above), then let's rename this to kudu_client_timer or so.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If0c793930799fdcaff53e705f94b52cadac2f53a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <aphadke@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message