You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Lars Volker (Code Review)" <ge...@cloudera.org> on 2019/03/21 21:53:45 UTC
[Impala-ASF-CR] IMPALA-7981: Add host disk usage to profile
Lars Volker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12822
Change subject: IMPALA-7981: Add host disk usage to profile
......................................................................
IMPALA-7981: Add host disk usage to profile
This change adds host disk usage to profiles. For each host that
participates in the query execution it adds the read and write bandwidth
across all disks. This includes all data read or written by the host as
part of the execution of a query (spilling), by the HDFS data node, and
by other processes running on the same system.
The change adds tests for the added functionality to the backend and end
to end tests.
Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
---
M be/src/runtime/query-state.cc
M be/src/util/disk-info.h
M be/src/util/system-state-info-test.cc
M be/src/util/system-state-info.cc
M be/src/util/system-state-info.h
M tests/query_test/test_observability.py
6 files changed, 236 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/12822/2
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 8: Code-Review+2
(2 comments)
Addressed the remaining comments in PS7, rebased in PS8, carrying Michael's +2
http://gerrit.cloudera.org:8080/#/c/12822/6/be/src/util/system-state-info.cc
File be/src/util/system-state-info.cc:
http://gerrit.cloudera.org:8080/#/c/12822/6/be/src/util/system-state-info.cc@57
PS6, Line 57: expected_n
> nit: expected_num_values
Done
http://gerrit.cloudera.org:8080/#/c/12822/6/be/src/util/system-state-info.cc@110
PS6, Line 110: }
:
> nit: unnecessary blank lines.
Done
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Apr 2019 20:33:36 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2679/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Apr 2019 20:53:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Apr 2019 05:58:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12822
to look at the new patch set (#4).
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
IMPALA-7981: Add host disk statistics to profile
This change adds host disk statistics to profiles. For each host that
participates in the query execution it adds the read and write bandwidth
across all disks. This includes all data read or written by the host as
part of the execution of a query (spilling), by the HDFS data node, and
by other processes running on the same system.
The change adds tests for the added functionality to the backend and end
to end tests.
Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
---
M be/src/runtime/query-state.cc
M be/src/util/disk-info.h
M be/src/util/system-state-info-test.cc
M be/src/util/system-state-info.cc
M be/src/util/system-state-info.h
M tests/query_test/test_observability.py
6 files changed, 245 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/12822/4
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-7981: Add host disk usage to profile
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk usage to profile
......................................................................
Patch Set 2:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info-test.cc
File be/src/util/system-state-info-test.cc:
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info-test.cc@105
PS2, Line 105: R"( 8 0 sda 17124835 222797 716029974 8414920 43758807 38432095 7867287712 630179264 0 32547524 638999340
> line too long (117 > 90)
Do we also care about testing for random devices which show up in /proc/diskstats such as ram0 ? My /proc/diskstats has the followings:
1 0 ram0 0 0 0 0 0 0 0 0 0 0 0
1 1 ram1 0 0 0 0 0 0 0 0 0 0 0
1 2 ram2 0 0 0 0 0 0 0 0 0 0 0
1 3 ram3 0 0 0 0 0 0 0 0 0 0 0
1 4 ram4 0 0 0 0 0 0 0 0 0 0 0
1 5 ram5 0 0 0 0 0 0 0 0 0 0 0
1 6 ram6 0 0 0 0 0 0 0 0 0 0 0
1 7 ram7 0 0 0 0 0 0 0 0 0 0 0
1 8 ram8 0 0 0 0 0 0 0 0 0 0 0
1 9 ram9 0 0 0 0 0 0 0 0 0 0 0
1 10 ram10 0 0 0 0 0 0 0 0 0 0 0
1 11 ram11 0 0 0 0 0 0 0 0 0 0 0
1 12 ram12 0 0 0 0 0 0 0 0 0 0 0
1 13 ram13 0 0 0 0 0 0 0 0 0 0 0
1 14 ram14 0 0 0 0 0 0 0 0 0 0 0
1 15 ram15 0 0 0 0 0 0 0 0 0 0 0
7 0 loop0 0 0 0 0 0 0 0 0 0 0 0
7 1 loop1 0 0 0 0 0 0 0 0 0 0 0
7 2 loop2 0 0 0 0 0 0 0 0 0 0 0
7 3 loop3 0 0 0 0 0 0 0 0 0 0 0
7 4 loop4 0 0 0 0 0 0 0 0 0 0 0
7 5 loop5 0 0 0 0 0 0 0 0 0 0 0
7 6 loop6 0 0 0 0 0 0 0 0 0 0 0
7 7 loop7 0 0 0 0 0 0 0 0 0 0 0
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info-test.cc@177
PS2, Line 177: // Tests the computation logic for disk usage. This test also makes sure that values of a
: // partition are not accounted towards the disk it is on.
Is this comment meant for the earlier test at line 103 ?
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.h
File be/src/util/system-state-info.h:
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.h@183
PS2, Line 183: /// The enum names correspond to the fields of /proc/diskstats
: /// https://www.kernel.org/doc/Documentation/iostats.txt
The followings are the metrics common to both 2.6 and 4.8+, right ?
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.h@221
PS2, Line 221: Entries are
: /// set to 0 for invalid entries.
Invalid entries are set to 0.
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.h@228
PS2, Line 228: /// Compute the disk usage.
Computes the read and write rate for the most recent epoch.
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.h@233
PS2, Line 233: DiskUsage
nit: I kind of interpret it as disk space used on first read. DiskStats may be a slightly better name but then it may not be consistent with other metrics we are measuring.
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.cc
File be/src/util/system-state-info.cc:
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.cc@45
PS2, Line 45: memset(&network_usage_, 0, sizeof(network_usage_));
memset(&disk_usage_, 0, sizeof(disk_usage_));
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.cc@81
PS2, Line 81: LOG(WARNING) << "Could not open " << path << ": " << GetStrErrMsg() << endl;
Will this lead to spam in the log if we periodically try reading the same path and fail or is it the responsibility of the caller to throttle calling this function ?
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.cc@199
PS2, Line 199:
DCHECK(disk_val_idx_ == 0 || disk_val_idx_ == 1);
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.cc@232
PS2, Line 232: ==
<= 0
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Sat, 06 Apr 2019 01:37:41 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12822/6/be/src/util/system-state-info-test.cc
File be/src/util/system-state-info-test.cc:
http://gerrit.cloudera.org:8080/#/c/12822/6/be/src/util/system-state-info-test.cc@122
PS6, Line 122: R"( 8 0 sda 17124835 222797 716029974 8414920 43758807 38432095 7867287712 630179264 0 32547524 638999340
line too long (117 > 90)
http://gerrit.cloudera.org:8080/#/c/12822/6/be/src/util/system-state-info-test.cc@123
PS6, Line 123: 8 1 sda1 17124482 222797 716027002 8414892 43546943 38432095 7867287712 629089180 0 31590972 637917344)";
line too long (118 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 6
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Apr 2019 19:04:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7981: Add host disk usage to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk usage to profile
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2512/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2019 22:20:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 10:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4008/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Apr 2019 05:58:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12822/4/be/src/util/system-state-info.h
File be/src/util/system-state-info.h:
http://gerrit.cloudera.org:8080/#/c/12822/4/be/src/util/system-state-info.h@255
PS4, Line 255: for (int i = 0; i < num_values; ++i) {
: int64_t v = -1;
: (*ss) >> v;
> Do you plan to choose https://gerrit.cloudera.org/#/c/12954/2/be/src/util/s
I think I prefer the one over there, but I don't feel strongly about either. Do you have a preference?
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Apr 2019 21:17:58 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 8:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4005/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Apr 2019 20:34:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12822/3/be/src/util/system-state-info-test.cc
File be/src/util/system-state-info-test.cc:
http://gerrit.cloudera.org:8080/#/c/12822/3/be/src/util/system-state-info-test.cc@106
PS3, Line 106: R"( 8 0 sda 17124835 222797 716029974 8414920 43758807 38432095 7867287712 630179264 0 32547524 638999340
line too long (117 > 90)
http://gerrit.cloudera.org:8080/#/c/12822/3/be/src/util/system-state-info-test.cc@107
PS3, Line 107: 8 1 sda1 17124482 222797 716027002 8414892 43546943 38432095 7867287712 629089180 0 31590972 637917344)";
line too long (118 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Apr 2019 20:06:45 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 9:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12822/9/be/src/util/system-state-info-test.cc
File be/src/util/system-state-info-test.cc:
http://gerrit.cloudera.org:8080/#/c/12822/9/be/src/util/system-state-info-test.cc@125
PS9, Line 125: R"( 8 0 $0 17124835 222797 716029974 8414920 43758807 38432095 7867287712 630179264 0 32547524 638999340
line too long (116 > 90)
http://gerrit.cloudera.org:8080/#/c/12822/9/be/src/util/system-state-info-test.cc@126
PS9, Line 126: 8 1 $01 17124482 222797 716027002 8414892 43546943 38432095 7867287712 629089180 0 31590972 637917344)";
line too long (117 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Apr 2019 05:58:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12822
to look at the new patch set (#9).
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
IMPALA-7981: Add host disk statistics to profile
This change adds host disk statistics to profiles. For each host that
participates in the query execution it adds the read and write bandwidth
across all disks. This includes all data read or written by the host as
part of the execution of a query (spilling), by the HDFS data node, and
by other processes running on the same system.
The change adds tests for the added functionality to the backend and end
to end tests.
Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
---
M be/src/runtime/query-state.cc
M be/src/util/disk-info.h
M be/src/util/system-state-info-test.cc
M be/src/util/system-state-info.cc
M be/src/util/system-state-info.h
M tests/query_test/test_observability.py
6 files changed, 266 insertions(+), 64 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/12822/9
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 6: Code-Review+2
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12822/6/be/src/util/system-state-info.cc
File be/src/util/system-state-info.cc:
http://gerrit.cloudera.org:8080/#/c/12822/6/be/src/util/system-state-info.cc@57
PS6, Line 57: num_values
nit: expected_num_values
http://gerrit.cloudera.org:8080/#/c/12822/6/be/src/util/system-state-info.cc@110
PS6, Line 110:
:
nit: unnecessary blank lines.
http://gerrit.cloudera.org:8080/#/c/12822/6/be/src/util/system-state-info.cc@129
PS6, Line 129: stat_string
Does it make sense to DCHECK(StringPiece(stat_string).starts_with('cpu ')); ?
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 6
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Apr 2019 19:10:44 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 9: Code-Review+2
Pulled the device names in the disk tests from DiskUtil because on the Jenkins workers, we don't have sda (but some different name). Carrying Michael's +2.
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Apr 2019 05:58:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
IMPALA-7981: Add host disk statistics to profile
This change adds host disk statistics to profiles. For each host that
participates in the query execution it adds the read and write bandwidth
across all disks. This includes all data read or written by the host as
part of the execution of a query (spilling), by the HDFS data node, and
by other processes running on the same system.
The change adds tests for the added functionality to the backend and end
to end tests.
Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Reviewed-on: http://gerrit.cloudera.org:8080/12822
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/runtime/query-state.cc
M be/src/util/disk-info.h
M be/src/util/system-state-info-test.cc
M be/src/util/system-state-info.cc
M be/src/util/system-state-info.h
M tests/query_test/test_observability.py
6 files changed, 266 insertions(+), 64 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 11
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12822
to look at the new patch set (#6).
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
IMPALA-7981: Add host disk statistics to profile
This change adds host disk statistics to profiles. For each host that
participates in the query execution it adds the read and write bandwidth
across all disks. This includes all data read or written by the host as
part of the execution of a query (spilling), by the HDFS data node, and
by other processes running on the same system.
The change adds tests for the added functionality to the backend and end
to end tests.
Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
---
M be/src/runtime/query-state.cc
M be/src/util/disk-info.h
M be/src/util/system-state-info-test.cc
M be/src/util/system-state-info.cc
M be/src/util/system-state-info.h
M tests/query_test/test_observability.py
6 files changed, 257 insertions(+), 64 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/12822/6
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 6
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12822/4/be/src/util/system-state-info.h
File be/src/util/system-state-info.h:
http://gerrit.cloudera.org:8080/#/c/12822/4/be/src/util/system-state-info.h@255
PS4, Line 255: for (int i = 0; i < num_values; ++i) {
: int64_t v = -1;
: (*ss) >> v;
Do you plan to choose https://gerrit.cloudera.org/#/c/12954/2/be/src/util/system-state-info.cc@169 over the current implementation ?
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.h
File be/src/util/system-state-info.h:
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.h@183
PS2, Line 183: /// The enum names correspond to the fields of /proc/diskstats
: /// https://www.kernel.org/doc/Documentation/iostats.txt
> These are the fields present in 2.6, while 4.8 added more fields that we're
Yes, I think the link to the doc should be sufficient.
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Apr 2019 20:56:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7981: Add host disk usage to profile
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12822
to look at the new patch set (#3).
Change subject: IMPALA-7981: Add host disk usage to profile
......................................................................
IMPALA-7981: Add host disk usage to profile
This change adds host disk usage to profiles. For each host that
participates in the query execution it adds the read and write bandwidth
across all disks. This includes all data read or written by the host as
part of the execution of a query (spilling), by the HDFS data node, and
by other processes running on the same system.
The change adds tests for the added functionality to the backend and end
to end tests.
Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
---
M be/src/runtime/query-state.cc
M be/src/util/disk-info.h
M be/src/util/system-state-info-test.cc
M be/src/util/system-state-info.cc
M be/src/util/system-state-info.h
M tests/query_test/test_observability.py
6 files changed, 245 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/12822/3
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12822/4/be/src/util/system-state-info.h
File be/src/util/system-state-info.h:
http://gerrit.cloudera.org:8080/#/c/12822/4/be/src/util/system-state-info.h@255
PS4, Line 255: for (int i = 0; i < num_values; ++i) {
: int64_t v = -1;
: (*ss) >> v;
> I think I prefer the one over there, but I don't feel strongly about either
On the other hand, this one uses standard c++ stringstreams so it might be easier to understand quickly.
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Apr 2019 21:19:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2706/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 6
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Apr 2019 19:35:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12822/7/be/src/util/system-state-info-test.cc
File be/src/util/system-state-info-test.cc:
http://gerrit.cloudera.org:8080/#/c/12822/7/be/src/util/system-state-info-test.cc@122
PS7, Line 122: R"( 8 0 sda 17124835 222797 716029974 8414920 43758807 38432095 7867287712 630179264 0 32547524 638999340
line too long (117 > 90)
http://gerrit.cloudera.org:8080/#/c/12822/7/be/src/util/system-state-info-test.cc@123
PS7, Line 123: 8 1 sda1 17124482 222797 716027002 8414892 43546943 38432095 7867287712 629089180 0 31590972 637917344)";
line too long (118 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Apr 2019 20:33:50 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12822
to look at the new patch set (#7).
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
IMPALA-7981: Add host disk statistics to profile
This change adds host disk statistics to profiles. For each host that
participates in the query execution it adds the read and write bandwidth
across all disks. This includes all data read or written by the host as
part of the execution of a query (spilling), by the HDFS data node, and
by other processes running on the same system.
The change adds tests for the added functionality to the backend and end
to end tests.
Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
---
M be/src/runtime/query-state.cc
M be/src/util/disk-info.h
M be/src/util/system-state-info-test.cc
M be/src/util/system-state-info.cc
M be/src/util/system-state-info.h
M tests/query_test/test_observability.py
6 files changed, 257 insertions(+), 64 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/12822/7
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2724/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Apr 2019 21:17:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7981: Add host disk usage to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk usage to profile
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info-test.cc
File be/src/util/system-state-info-test.cc:
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info-test.cc@105
PS2, Line 105: R"( 8 0 sda 17124835 222797 716029974 8414920 43758807 38432095 7867287712 630179264 0 32547524 638999340
line too long (117 > 90)
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info-test.cc@106
PS2, Line 106: 8 1 sda1 17124482 222797 716027002 8414892 43546943 38432095 7867287712 629089180 0 31590972 637917344)";
line too long (118 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2019 21:54:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7981: Add host disk usage to profile
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk usage to profile
......................................................................
Patch Set 3:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info-test.cc
File be/src/util/system-state-info-test.cc:
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info-test.cc@105
PS2, Line 105: string disk_stats =
> Do we also care about testing for random devices which show up in /proc/dis
I added two of these lines but the code will ignore anything that has not been identified by DiskInfo, and then 0 values will not to the result.
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info-test.cc@177
PS2, Line 177:
: // Tests the computation logic for disk statistics.
> Is this comment meant for the earlier test at line 103 ?
Yes, thx.
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.h
File be/src/util/system-state-info.h:
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.h@183
PS2, Line 183: /// The enum names correspond to the fields of /proc/diskstats
: /// https://www.kernel.org/doc/Documentation/iostats.txt
> The followings are the metrics common to both 2.6 and 4.8+, right ?
These are the fields present in 2.6, while 4.8 added more fields that we're not using. Would you like me to add a comment elaborating this? It's also explained in the linked doc.
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.h@221
PS2, Line 221: Invalid
: /// entries are set to 0.
> Invalid entries are set to 0.
Done
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.h@228
PS2, Line 228: /// Computes the read and w
> Computes the read and write rate for the most recent epoch.
Done
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.h@233
PS2, Line 233: DiskStats
> nit: I kind of interpret it as disk space used on first read. DiskStats may
I switched to DiskStats.
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.cc
File be/src/util/system-state-info.cc:
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.cc@45
PS2, Line 45: memset(&cpu_ratios_, 0, sizeof(cpu_ratios_));
> memset(&disk_usage_, 0, sizeof(disk_usage_));
Done
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.cc@81
PS2, Line 81: du::Status status = ReadFileToString(Env::Default(), path, buf);
> Will this lead to spam in the log if we periodically try reading the same p
Done
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.cc@199
PS2, Line 199: }
> DCHECK(disk_val_idx_ == 0 || disk_val_idx_ == 1);
Done
http://gerrit.cloudera.org:8080/#/c/12822/2/be/src/util/system-state-info.cc@232
PS2, Line 232:
> <= 0
Done
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Apr 2019 20:06:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2732/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Apr 2019 06:30:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 10: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Apr 2019 11:05:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 6:
(1 comment)
Rebased the change and made the overall code a bit more consistent. Please see PS6.
http://gerrit.cloudera.org:8080/#/c/12822/4/be/src/util/system-state-info.h
File be/src/util/system-state-info.h:
http://gerrit.cloudera.org:8080/#/c/12822/4/be/src/util/system-state-info.h@255
PS4, Line 255:
:
:
> On the other hand, this one uses standard c++ stringstreams so it might be
Went with stringpieces and moved this to an anonymous namespace in the .cc file.
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 6
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Apr 2019 19:03:49 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7981: Add host disk statistics to profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12822 )
Change subject: IMPALA-7981: Add host disk statistics to profile
......................................................................
Patch Set 8: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4005/
--
To view, visit http://gerrit.cloudera.org:8080/12822
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373e7da47a0d722938e6ca1572c49a502951ed57
Gerrit-Change-Number: 12822
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Apr 2019 01:47:56 +0000
Gerrit-HasComments: No