[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: anujphadke Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: anujphadke
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: anujphadke Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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:aee4f2c50001 (host=anuj-OptiPlex-9020:22000):(Total: 12.615ms, non-child: 0.000ns, % non-child: 0.00%) Hdfs split stats (:<# splits>/): - 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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: anujphadke Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Matthew Jacobs
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Matthew Jacobs
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node
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 Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: Yes
[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
