[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-07 Thread Lars Volker (Code Review)
Lars Volker has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12954


Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..

IMPALA-8395: Parse older formats of /proc/net/dev correctly

Older kernel versions don't have a space between the interface name and
the first counter value in /proc/net/dev. This change reworks the
parsing logic to support such older formats and adds a unit test for it.

Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
---
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
3 files changed, 32 insertions(+), 14 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/12954/1/be/src/util/system-state-info-test.cc
File be/src/util/system-state-info-test.cc:

http://gerrit.cloudera.org:8080/#/c/12954/1/be/src/util/system-state-info-test.cc@96
PS1, Line 96:   string dev_net = R"(Inter-|   Receive   
 |  Transmit
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/12954/1/be/src/util/system-state-info-test.cc@97
PS1, Line 97:  face |bytespackets errs drop fifo frame compressed 
multicast|bytespackets errs drop fifo colls carrier compressed
line too long (122 > 90)


http://gerrit.cloudera.org:8080/#/c/12954/1/be/src/util/system-state-info-test.cc@98
PS1, Line 98: lo:5829  53000 0  0 0 
5829  53000 0   0  0
line too long (122 > 90)


http://gerrit.cloudera.org:8080/#/c/12954/1/be/src/util/system-state-info-test.cc@99
PS1, Line 99:   eth0:285025090  212208000 0  0 
0  9137793   84770000 0   0  0)";
line too long (126 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Sun, 07 Apr 2019 20:56:25 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/2670/ : 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/12954
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Sun, 07 Apr 2019 21:38:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Bharath Vissapragada (Code Review)
Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 1: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12954/1/be/src/util/system-state-info-test.cc
File be/src/util/system-state-info-test.cc:

http://gerrit.cloudera.org:8080/#/c/12954/1/be/src/util/system-state-info-test.cc@80
PS1, Line 80:face |bytespackets errs drop fifo frame compressed 
multicast|bytespackets errs drop fifo colls carrier compressed
nit: I think you could also dump these contents to some files and read them 
back in the test.


http://gerrit.cloudera.org:8080/#/c/12954/1/be/src/util/system-state-info.cc
File be/src/util/system-state-info.cc:

http://gerrit.cloudera.org:8080/#/c/12954/1/be/src/util/system-state-info.cc@172
PS1, Line 172: *v = 0;
nit: trace log something?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 08 Apr 2019 17:44:45 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Lars Volker (Code Review)
Hello Bharath Vissapragada, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..

IMPALA-8395: Parse older formats of /proc/net/dev correctly

Older kernel versions don't have a space between the interface name and
the first counter value in /proc/net/dev. This change reworks the
parsing logic to support such older formats and adds a unit test for it.

Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
---
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
3 files changed, 36 insertions(+), 14 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 2: Code-Review+1

(2 comments)

Carrying Bharath's +1.

http://gerrit.cloudera.org:8080/#/c/12954/1/be/src/util/system-state-info-test.cc
File be/src/util/system-state-info-test.cc:

http://gerrit.cloudera.org:8080/#/c/12954/1/be/src/util/system-state-info-test.cc@80
PS1, Line 80:face |bytespackets errs drop fifo frame compressed 
multicast|bytespackets errs drop fifo colls carrier compressed
> nit: I think you could also dump these contents to some files and read them
Do we currently do this in other tests? I like that they're are inlined here 
which prevents an additional indirection, but I agree that the long lines are 
not ideal. Let me know if you feel strongly about it, otherwise I'd keep them 
here.


http://gerrit.cloudera.org:8080/#/c/12954/1/be/src/util/system-state-info.cc
File be/src/util/system-state-info.cc:

http://gerrit.cloudera.org:8080/#/c/12954/1/be/src/util/system-state-info.cc@172
PS1, Line 172:
> nit: trace log something?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Mon, 08 Apr 2019 20:00:19 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/12954/2/be/src/util/system-state-info-test.cc
File be/src/util/system-state-info-test.cc:

http://gerrit.cloudera.org:8080/#/c/12954/2/be/src/util/system-state-info-test.cc@96
PS2, Line 96:   string dev_net = R"(Inter-|   Receive   
 |  Transmit
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/12954/2/be/src/util/system-state-info-test.cc@97
PS2, Line 97:  face |bytespackets errs drop fifo frame compressed 
multicast|bytespackets errs drop fifo colls carrier compressed
line too long (122 > 90)


http://gerrit.cloudera.org:8080/#/c/12954/2/be/src/util/system-state-info-test.cc@98
PS2, Line 98: lo:5829  53000 0  0 0 
5829  53000 0   0  0
line too long (122 > 90)


http://gerrit.cloudera.org:8080/#/c/12954/2/be/src/util/system-state-info-test.cc@99
PS2, Line 99:   eth0:285025090  212208000 0  0 
0  9137793   84770000 0   0  0)";
line too long (126 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Mon, 08 Apr 2019 20:01:00 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Bharath Vissapragada (Code Review)
Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12954/1/be/src/util/system-state-info-test.cc
File be/src/util/system-state-info-test.cc:

http://gerrit.cloudera.org:8080/#/c/12954/1/be/src/util/system-state-info-test.cc@80
PS1, Line 80:face |bytespackets errs drop fifo frame compressed 
multicast|bytespackets errs drop fifo colls carrier compressed
> Do we currently do this in other tests? I like that they're are inlined her
Not too strong, I thought putting in a file means better readability. Don't see 
places where we exactly do something like this for be-tests but we do store the 
certs and pem files for thrift-server-test in be/src/testutil. Your call, like 
I mentioned, not too strong.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Mon, 08 Apr 2019 20:24:24 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12954/2/be/src/util/system-state-info.cc
File be/src/util/system-state-info.cc:

http://gerrit.cloudera.org:8080/#/c/12954/2/be/src/util/system-state-info.cc@168
PS2, Line 168: colon_idx = sp.find_first_of(':');
Should this also check for the case of colon_idx == npos and skip ?


http://gerrit.cloudera.org:8080/#/c/12954/2/be/src/util/system-state-info.cc@170
PS2, Line 170:
I wonder if we should just skip if counters.values() < NUM_NET_VALUES ? Will 
doing so cause us to always skip for certain versions of old kernels ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 08 Apr 2019 20:38:47 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 2:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/2676/ : 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/12954
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 08 Apr 2019 20:41:22 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Lars Volker (Code Review)
Hello Bharath Vissapragada, Michael Ho, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..

IMPALA-8395: Parse older formats of /proc/net/dev correctly

Older kernel versions don't have a space between the interface name and
the first counter value in /proc/net/dev. This change reworks the
parsing logic to support such older formats and adds a unit test for it.

Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
---
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
3 files changed, 40 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12954/2/be/src/util/system-state-info.cc
File be/src/util/system-state-info.cc:

http://gerrit.cloudera.org:8080/#/c/12954/2/be/src/util/system-state-info.cc@168
PS2, Line 168: colon_idx = sp.find_first_of(':');
> Should this also check for the case of colon_idx == npos and skip ?
Counters will come up empty in this case because substr() will return an empty 
StringPiece. Should I explain this in a comment?


http://gerrit.cloudera.org:8080/#/c/12954/2/be/src/util/system-state-info.cc@170
PS2, Line 170:
> I wonder if we should just skip if counters.values() < NUM_NET_VALUES ? Wil
Done. I checked Centos6 and the manpage for proc lists the same fields.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 08 Apr 2019 21:16:22 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/12954/3/be/src/util/system-state-info-test.cc
File be/src/util/system-state-info-test.cc:

http://gerrit.cloudera.org:8080/#/c/12954/3/be/src/util/system-state-info-test.cc@96
PS3, Line 96:   string dev_net = R"(Inter-|   Receive   
 |  Transmit
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/12954/3/be/src/util/system-state-info-test.cc@97
PS3, Line 97:  face |bytespackets errs drop fifo frame compressed 
multicast|bytespackets errs drop fifo colls carrier compressed
line too long (122 > 90)


http://gerrit.cloudera.org:8080/#/c/12954/3/be/src/util/system-state-info-test.cc@98
PS3, Line 98: lo:5829  53000 0  0 0 
5829  53000 0   0  0
line too long (122 > 90)


http://gerrit.cloudera.org:8080/#/c/12954/3/be/src/util/system-state-info-test.cc@99
PS3, Line 99:   eth0:285025090  212208000 0  0 
0  9137793   84770000 0   0  0)";
line too long (126 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 08 Apr 2019 21:17:11 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/2683/ : 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/12954
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 08 Apr 2019 21:45:49 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Lars Volker (Code Review)
Hello Bharath Vissapragada, Michael Ho, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..

IMPALA-8395: Parse older formats of /proc/net/dev correctly

Older kernel versions don't have a space between the interface name and
the first counter value in /proc/net/dev. This change reworks the
parsing logic to support such older formats and adds a unit test for it.

Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
---
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
3 files changed, 46 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/12954/4/be/src/util/system-state-info-test.cc
File be/src/util/system-state-info-test.cc:

http://gerrit.cloudera.org:8080/#/c/12954/4/be/src/util/system-state-info-test.cc@96
PS4, Line 96:   string dev_net = R"(Inter-|   Receive   
 |  Transmit
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/12954/4/be/src/util/system-state-info-test.cc@97
PS4, Line 97:  face |bytespackets errs drop fifo frame compressed 
multicast|bytespackets errs drop fifo colls carrier compressed
line too long (122 > 90)


http://gerrit.cloudera.org:8080/#/c/12954/4/be/src/util/system-state-info-test.cc@98
PS4, Line 98: lo:5829  53000 0  0 0 
5829  53000 0   0  0
line too long (122 > 90)


http://gerrit.cloudera.org:8080/#/c/12954/4/be/src/util/system-state-info-test.cc@99
PS4, Line 99:   eth0:285025090  212208000 0  0 
0  9137793   84770000 0   0  0)";
line too long (126 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 08 Apr 2019 22:23:58 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12954/2/be/src/util/system-state-info.cc
File be/src/util/system-state-info.cc:

http://gerrit.cloudera.org:8080/#/c/12954/2/be/src/util/system-state-info.cc@168
PS2, Line 168: colon_idx = sp.find_first_of(':');
> Counters will come up empty in this case because substr() will return an em
I was wrong, counters will contain the whole line. I added a check, not finding 
a colon is clearly a sign of a problem.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 08 Apr 2019 22:24:07 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/2689/ : 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/12954
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 08 Apr 2019 23:05:09 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Tue, 09 Apr 2019 02:05:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Tue, 09 Apr 2019 05:24:16 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4000/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Tue, 09 Apr 2019 05:24:17 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-09 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..

IMPALA-8395: Parse older formats of /proc/net/dev correctly

Older kernel versions don't have a space between the interface name and
the first counter value in /proc/net/dev. This change reworks the
parsing logic to support such older formats and adds a unit test for it.

Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Reviewed-on: http://gerrit.cloudera.org:8080/12954
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
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
3 files changed, 46 insertions(+), 11 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 6
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 


[Impala-ASF-CR] IMPALA-8395: Parse older formats of /proc/net/dev correctly

2019-04-09 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12954 )

Change subject: IMPALA-8395: Parse older formats of /proc/net/dev correctly
..


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Gerrit-Change-Number: 12954
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Tue, 09 Apr 2019 10:31:13 +
Gerrit-HasComments: No