You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "anujphadke (Code Review)" <ge...@cloudera.org> on 2016/10/06 00:18:42 UTC

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node 

anujphadke has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4639

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

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

Currently we do not start the TotalStorageWaitTime timer in the
kudu-scanner. This patch starts the timer before calling GetNextScannerBatch
to indicate how much time was spent waiting on data from kudu.

Change-Id: If0c793930799fdcaff53e705f94b52cadac2f53a
---
M be/src/exec/kudu-scanner.cc
1 file changed, 1 insertion(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/4639/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: If0c793930799fdcaff53e705f94b52cadac2f53a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <ap...@cloudera.com>

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

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


Patch Set 3: Verified+1

-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has submitted this change and it was merged.

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


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

Currently we do not start the TotalStorageWaitTime timer in the
kudu-scanner. This patch replaces the kudu_read_timer with the
TotalStorageWaitTime which measures the intended time.

Change-Id: If0c793930799fdcaff53e705f94b52cadac2f53a
Reviewed-on: http://gerrit.cloudera.org:8080/4639
Reviewed-by: Matthew Jacobs <mj...@cloudera.com>
Tested-by: Internal Jenkins
---
M be/src/exec/kudu-scan-node.cc
M be/src/exec/kudu-scan-node.h
M be/src/exec/kudu-scanner.cc
3 files changed, 2 insertions(+), 8 deletions(-)

Approvals:
  Matthew Jacobs: Looks good to me, approved
  Internal Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: If0c793930799fdcaff53e705f94b52cadac2f53a
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: anujphadke <ap...@cloudera.com>

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "anujphadke (Code Review)" <ge...@cloudera.org>.
anujphadke has posted comments on this change.

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


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4639/3/be/src/exec/kudu-scan-node.h
File be/src/exec/kudu-scan-node.h:

PS3, Line 104:   static const std::string KUDU_ROUND_TRIPS
> *added back
Sorry these 2 lines came back after a rebase. There was a conflict.
Fixed it.

Ran a simple select * on a kudu table and verified it again that the TotalStorageWaitTime reports a time in profile (after rebasing)

  Fragment F00:
      Instance 254e4c4f30d99581:aee4f2c500000001 (host=anuj-OptiPlex-9020:22000):(Total: 12.615ms, non-child: 0.000ns, % non-child: 0.00%)
        Hdfs split stats (<volume id>:<# splits>/<split lengths>): 
         - AverageThreadTokens: 0.00 
         - BloomFilterBytes: 0
         - ExecTreePrepareTime: 201.421us
         - PeakMemoryUsage: 245.12 KB (251000)
         - PerHostPeakMemUsage: 273.12 KB (279680)
         - RowsProduced: 8 (8)
         - TotalCpuTime: 64.250ms
         - TotalNetworkReceiveTime: 0.000ns
         - TotalNetworkSendTime: 597.148us
         - TotalStorageWaitTime: 55.906ms


-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: anujphadke <ap...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
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:

Thanks, Dan.

In that case:
Anuj, it looks like we can remove the counter we have and use TotalStorageWaitTime wherever we were calling the previous one.

-- 
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 <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

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


Patch Set 2: Code-Review+1

Thanks!

-- 
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: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

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


Patch Set 3: -Code-Review

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4639/3/be/src/exec/kudu-scan-node.h
File be/src/exec/kudu-scan-node.h:

PS3, Line 104:   static const std::string KUDU_READ_TIMER;
why was this added? It doesn't look like it's used


-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

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


Patch Set 1:

(2 comments)

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?
This operation doesn't require any RPCs, so it should be fast.


PS1, Line 159: scanner_->Close();
> @Kudu-team: Is this worth accounting for?
If the scan node doesn't scan until completion, calling Close may issue an async RPC to the tserver to close the scanner.  However, I doubt y'all don't finish the scan, and the RPC is not waited on, so this should be fast.


-- 
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 <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

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


Patch Set 3: Code-Review+2

-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "anujphadke (Code Review)" <ge...@cloudera.org>.
Hello Matthew Jacobs, Dan Hecht,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/4639

to look at the new patch set (#3).

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

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

Currently we do not start the TotalStorageWaitTime timer in the
kudu-scanner. This patch replaces the kudu_read_timer with the
TotalStorageWaitTime which measures the intended time.

Change-Id: If0c793930799fdcaff53e705f94b52cadac2f53a
---
M be/src/exec/kudu-scan-node.cc
M be/src/exec/kudu-scan-node.h
M be/src/exec/kudu-scanner.cc
3 files changed, 2 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/4639/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c793930799fdcaff53e705f94b52cadac2f53a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "anujphadke (Code Review)" <ge...@cloudera.org>.
anujphadke has uploaded a new patch set (#2).

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

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

Currently we do not start the TotalStorageWaitTime timer in the
kudu-scanner. This patch replaces the kudu_read_timer with the
TotalStorageWaitTime which measures the intended time.

Change-Id: If0c793930799fdcaff53e705f94b52cadac2f53a
---
M be/src/exec/kudu-scan-node.cc
M be/src/exec/kudu-scan-node.h
M be/src/exec/kudu-scanner.cc
3 files changed, 2 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/4639/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c793930799fdcaff53e705f94b52cadac2f53a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
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 <ap...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

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


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4639/3/be/src/exec/kudu-scan-node.h
File be/src/exec/kudu-scan-node.h:

PS3, Line 104:   static const std::string KUDU_READ_TIMER;
> why was this added? It doesn't look like it's used
*added back

It wasn't in rev 2, but is back again in rev3.


-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

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


Patch Set 4: Code-Review+2

Thanks for fixing that. Can you just submit it through gvm again?

-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: anujphadke <ap...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
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:

(1 comment)

Thanks Dan, 1 more below...

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

PS1, Line 152: scanner_->Open()
@Dan, I meant to call this one out as well.


-- 
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 <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

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


Patch Set 4: Verified+1

-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: anujphadke <ap...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "anujphadke (Code Review)" <ge...@cloudera.org>.
Hello Matthew Jacobs, Internal Jenkins, Dan Hecht,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/4639

to look at the new patch set (#4).

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

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

Currently we do not start the TotalStorageWaitTime timer in the
kudu-scanner. This patch replaces the kudu_read_timer with the
TotalStorageWaitTime which measures the intended time.

Change-Id: If0c793930799fdcaff53e705f94b52cadac2f53a
---
M be/src/exec/kudu-scan-node.cc
M be/src/exec/kudu-scan-node.h
M be/src/exec/kudu-scanner.cc
3 files changed, 2 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/4639/4
-- 
To view, visit http://gerrit.cloudera.org:8080/4639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c793930799fdcaff53e705f94b52cadac2f53a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

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


Patch Set 2: Code-Review+2

-- 
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: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

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


Patch Set 1:

(1 comment)

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

PS1, Line 152: scanner_->Open()
> @Dan, I meant to call this one out as well.
Open() will fetch the first batch of data via RPC, so it should be counted.


-- 
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 <ap...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes