[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

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

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 14: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 Jan 2019 10:44:49 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

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

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..

IMPALA-7941: part 1: detect cgroups memory limit

This adds the logic to detect the cgroups memory limit,
but does not use it to set the process memory limit yet.
The information obtained is logged at startup and accessible
through the Web UI.

The patch boils down to reading from the appropriate
places in the filesystem. The main complication is that paths need to be
translated to point to the right cgroup node inside the container.

This deletes some useless cgroup logic from ProcessStateInfo that
printed whatever happened to be the last cgroup in a file.

Testing:
Added a unit test to check that the code could parse the cgroups
hierarchy ok and return a positive value.

Tested on CentOS6 and CentOS7.

Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Reviewed-on: http://gerrit.cloudera.org:8080/12120
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/common/init.cc
M be/src/util/CMakeLists.txt
A be/src/util/cgroup-util.cc
A be/src/util/cgroup-util.h
M be/src/util/default-path-handlers.cc
M be/src/util/proc-info-test.cc
M be/src/util/process-state-info.cc
M be/src/util/process-state-info.h
M www/root.tmpl
9 files changed, 289 insertions(+), 52 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 15
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

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

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 14:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 Jan 2019 06:40:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

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

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 14: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 Jan 2019 06:40:04 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

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

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 13:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 Jan 2019 01:54:44 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-22 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12120 )

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 13: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 Jan 2019 01:38:33 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-22 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12120 )

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12120/9/be/src/util/cgroup-util.cc
File be/src/util/cgroup-util.cc:

http://gerrit.cloudera.org:8080/#/c/12120/9/be/src/util/cgroup-util.cc@55
PS9, Line 55: if (!proc_cgroups.good()) continue;
> i think at this point it ll be an overkill to parse this, maybe just return
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 Jan 2019 01:14:04 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-22 Thread Tim Armstrong (Code Review)
Hello Pooja Nilangekar, Philip Zeyliger, Bikramjeet Vig, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..

IMPALA-7941: part 1: detect cgroups memory limit

This adds the logic to detect the cgroups memory limit,
but does not use it to set the process memory limit yet.
The information obtained is logged at startup and accessible
through the Web UI.

The patch boils down to reading from the appropriate
places in the filesystem. The main complication is that paths need to be
translated to point to the right cgroup node inside the container.

This deletes some useless cgroup logic from ProcessStateInfo that
printed whatever happened to be the last cgroup in a file.

Testing:
Added a unit test to check that the code could parse the cgroups
hierarchy ok and return a positive value.

Tested on CentOS6 and CentOS7.

Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
---
M be/src/common/init.cc
M be/src/util/CMakeLists.txt
A be/src/util/cgroup-util.cc
A be/src/util/cgroup-util.h
M be/src/util/default-path-handlers.cc
M be/src/util/proc-info-test.cc
M be/src/util/process-state-info.cc
M be/src/util/process-state-info.h
M www/root.tmpl
9 files changed, 289 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/12120/13
--
To view, visit http://gerrit.cloudera.org:8080/12120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-22 Thread Tim Armstrong (Code Review)
Hello Pooja Nilangekar, Philip Zeyliger, Bikramjeet Vig, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..

IMPALA-7941: part 1: detect cgroups memory limit

This adds the logic to detect the cgroups memory limit,
but does not use it to set the process memory limit yet.
The information obtained is logged at startup and accessible
through the Web UI.

The patch boils down to reading from the appropriate
places in the filesystem. The main complication is that paths need to be
translated to point to the right cgroup node inside the container.

This deletes some useless cgroup logic from ProcessStateInfo that
printed whatever happened to be the last cgroup in a file.

Testing:
Added a unit test to check that the code could parse the cgroups
hierarchy ok and return a positive value.

Tested on CentOS6 and CentOS7.

Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
---
M be/src/common/init.cc
M be/src/util/CMakeLists.txt
A be/src/util/cgroup-util.cc
A be/src/util/cgroup-util.h
M be/src/util/default-path-handlers.cc
M be/src/util/proc-info-test.cc
M be/src/util/process-state-info.cc
M be/src/util/process-state-info.h
M www/root.tmpl
9 files changed, 285 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/12120/12
--
To view, visit http://gerrit.cloudera.org:8080/12120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-22 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12120 )

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12120/9/be/src/util/cgroup-util.cc
File be/src/util/cgroup-util.cc:

http://gerrit.cloudera.org:8080/#/c/12120/9/be/src/util/cgroup-util.cc@55
PS9, Line 55: if (!proc_cgroups.good()) continue;
> I also checked this and it doesn't look like : is escaped in the paths. Not
i think at this point it ll be an overkill to parse this, maybe just return an 
error if fields.size() > 3.

Another case that we dont seem to tackle here is when multiple subsystems are 
attached to the same cgroup hierarchy and the corresponding line will look like 
the example you just mentioned above: '9:cpu,cpuacct:/user.slice/foo:bar'. For 
this case we will have to split fields[1] on  ',' too and look for the 
subsystem OR use string::find



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 Jan 2019 00:45:40 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

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

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 10:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 10
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 22 Jan 2019 20:44:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-22 Thread Tim Armstrong (Code Review)
Hello Pooja Nilangekar, Philip Zeyliger, Bikramjeet Vig, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..

IMPALA-7941: part 1: detect cgroups memory limit

This adds the logic to detect the cgroups memory limit,
but does not use it to set the process memory limit yet.
The information obtained is logged at startup and accessible
through the Web UI.

The patch boils down to reading from the appropriate
places in the filesystem. The main complication is that paths need to be
translated to point to the right cgroup node inside the container.

This deletes some useless cgroup logic from ProcessStateInfo that
printed whatever happened to be the last cgroup in a file.

Testing:
Added a unit test to check that the code could parse the cgroups
hierarchy ok and return a positive value.

Tested on CentOS6 and CentOS7.

Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
---
M be/src/common/init.cc
M be/src/util/CMakeLists.txt
A be/src/util/cgroup-util.cc
A be/src/util/cgroup-util.h
M be/src/util/default-path-handlers.cc
M be/src/util/proc-info-test.cc
M be/src/util/process-state-info.cc
M be/src/util/process-state-info.h
M www/root.tmpl
9 files changed, 278 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/12120/10
--
To view, visit http://gerrit.cloudera.org:8080/12120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 10
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-22 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12120 )

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc
File be/src/util/cgroup-util.cc:

http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc@86
PS7, Line 86: split(fields, line, is_any_of(" "), token_compress_on);
> https://superuser.com/a/527503
Done - used a gutil string escaping function


http://gerrit.cloudera.org:8080/#/c/12120/9/be/src/util/cgroup-util.cc
File be/src/util/cgroup-util.cc:

http://gerrit.cloudera.org:8080/#/c/12120/9/be/src/util/cgroup-util.cc@55
PS9, Line 55: split(fields, line, is_any_of(":"));
I also checked this and it doesn't look like : is escaped in the paths. Not 
sure how much we can reasonably do here:

  9:cpu,cpuacct:/user.slice/foo:bar



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 22 Jan 2019 19:37:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-17 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12120 )

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 9:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc
File be/src/util/cgroup-util.cc:

http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc@86
PS7, Line 86: split(fields, line, is_any_of(" "), token_compress_on);
> Can you send me a pointer to the info you found about this? It seems like w
https://superuser.com/a/527503

I tried this out myself by running:
sudo  mount /dev/sda3 /home/user/mnt\ check/

then checked output of mountinfo: "more /proc/self/mountinfo"
output:
305 29 8:3 / /home/user/mnt\040check rw,relatime shared:137 - ext4 /dev/sda3 
rw,data=ordered


http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc@86
PS7, Line 86: split(fields, line, is_any_of(" "), token_compress_on);
> That's a good point. I guess this won't work correctly in that case since w
You are right, it wont work correctly in that case, we will have to decode the 
escape characters since they would be written down in octal but wont be 
accepted as a valid file name when trying to read it



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 18 Jan 2019 02:32:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-17 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12120 )

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 9:

(1 comment)

Any chance to look at my question?

http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc
File be/src/util/cgroup-util.cc:

http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc@86
PS7, Line 86: split(fields, line, is_any_of(" "), token_compress_on);
> i was curious as to how this will work if the mount point had spaces in it.
That's a good point. I guess this won't work correctly in that case since we're 
not decon



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 18 Jan 2019 01:46:12 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

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

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 9:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 12 Jan 2019 00:41:53 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

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

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 8:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 12 Jan 2019 00:13:43 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-11 Thread Tim Armstrong (Code Review)
Hello Pooja Nilangekar, Philip Zeyliger, Bikramjeet Vig, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..

IMPALA-7941: part 1: detect cgroups memory limit

This adds the logic to detect the cgroups memory limit,
but does not use it to set the process memory limit yet.
The information obtained is logged at startup and accessible
through the Web UI.

The patch boils down to reading from the appropriate
places in the filesystem. The main complication is that paths need to be
translated to point to the right cgroup node inside the container.

This deletes some useless cgroup logic from ProcessStateInfo that
printed whatever happened to be the last cgroup in a file.

Testing:
Added a unit test to check that the code could parse the cgroups
hierarchy ok and return a positive value.

Tested on CentOS6 and CentOS7.

Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
---
M be/src/common/init.cc
M be/src/util/CMakeLists.txt
A be/src/util/cgroup-util.cc
A be/src/util/cgroup-util.h
M be/src/util/default-path-handlers.cc
M be/src/util/proc-info-test.cc
M be/src/util/process-state-info.cc
M be/src/util/process-state-info.h
M www/root.tmpl
9 files changed, 267 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/12120/9
--
To view, visit http://gerrit.cloudera.org:8080/12120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12120 )

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 7:

(4 comments)

Fixed a few things. Have a follow-on Q about the escapes

http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc
File be/src/util/cgroup-util.cc:

http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc@40
PS7, Line 40: cgroup_type
Per discussion, subsystem is a more accurate term.


http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc@86
PS7, Line 86: split(fields, line, is_any_of(" "), token_compress_on);
> i was curious as to how this will work if the mount point had spaces in it.
This still isn't likely to work correctly since we'd need to decode it to get a 
valid path.


http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc@86
PS7, Line 86: split(fields, line, is_any_of(" "), token_compress_on);
> i was curious as to how this will work if the mount point had spaces in it.
Can you send me a pointer to the info you found about this? It seems like we 
should, in principle, unescape the string so that later operations on it will 
work but I wanted to confirm the format. gutil has some utilities like 
CUnescape() that might be good enough for this.

It does seem kinda like asking for trouble if someone configures things so that 
there are spaces in these paths though.


http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc@112
PS7, Line 112: path->compare(0, system_path.size(), system_path) == 0
> is there a case where this might not be true?
Erm, that's a good point. It would be weird if it didn't. I'll return an error 
if this isn't the case.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 11 Jan 2019 23:39:29 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-11 Thread Tim Armstrong (Code Review)
Hello Pooja Nilangekar, Philip Zeyliger, Bikramjeet Vig, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..

IMPALA-7941: part 1: detect cgroups memory limit

This adds the logic to detect the cgroups memory limit,
but does not use it to set the process memory limit yet.
The information obtained is logged at startup and accessible
through the Web UI.

The patch boils down to reading from the appropriate
places in the filesystem. The main complication is that paths need to be
translated to point to the right cgroup node inside the container.

This deletes some useless cgroup logic from ProcessStateInfo that
printed whatever happened to be the last cgroup in a file.

Testing:
Added a unit test to check that the code could parse the cgroups
hierarchy ok and return a positive value.

Tested on CentOS6 and CentOS7.

Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
---
M be/src/common/init.cc
M be/src/util/CMakeLists.txt
A be/src/util/cgroup-util.cc
A be/src/util/cgroup-util.h
M be/src/util/default-path-handlers.cc
M be/src/util/proc-info-test.cc
M be/src/util/process-state-info.cc
M be/src/util/process-state-info.h
M www/root.tmpl
9 files changed, 267 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/12120/8
--
To view, visit http://gerrit.cloudera.org:8080/12120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-11 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12120 )

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc
File be/src/util/cgroup-util.cc:

http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc@86
PS7, Line 86: split(fields, line, is_any_of(" "), token_compress_on);
i was curious as to how this will work if the mount point had spaces in it. 
looks like it will output the ascii code in octal for a space instead of a "\ 
", so should be fine.


http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc@112
PS7, Line 112: path->compare(0, system_path.size(), system_path) == 0
is there a case where this might not be true?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 11 Jan 2019 18:58:33 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-04 Thread Pooja Nilangekar (Code Review)
Pooja Nilangekar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12120 )

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 7: Code-Review+1

(1 comment)

> Patch Set 6:
>
> (5 comments)

http://gerrit.cloudera.org:8080/#/c/12120/6/be/src/util/cgroup-util.cc
File be/src/util/cgroup-util.cc:

http://gerrit.cloudera.org:8080/#/c/12120/6/be/src/util/cgroup-util.cc@111
PS6, Line 111: em_path = paths.second;
> My reading of the std::string docs are that it should work in either case (
I didn't know this. Thanks for the explanation and the example!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 04 Jan 2019 18:26:29 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

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

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 7:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 04 Jan 2019 01:29:07 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-03 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12120 )

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/12120/6/be/src/util/cgroup-util.h
File be/src/util/cgroup-util.h:

http://gerrit.cloudera.org:8080/#/c/12120/6/be/src/util/cgroup-util.h@43
PS6, Line 43: is a
> Nit: extra words?
Done


http://gerrit.cloudera.org:8080/#/c/12120/6/be/src/util/cgroup-util.cc
File be/src/util/cgroup-util.cc:

http://gerrit.cloudera.org:8080/#/c/12120/6/be/src/util/cgroup-util.cc@108
PS6, Line 108:   pair paths;
I got confused by which path was which - a bad sign when it's my own code. I 
introduced some intermediate variables to hopefully make it more readable.


http://gerrit.cloudera.org:8080/#/c/12120/6/be/src/util/cgroup-util.cc@111
PS6, Line 111:  paths.second.size(), paths.first
> I am probably missing something here, would this cause an issue if the leng
My reading of the std::string docs are that it should work in either case 
(reallocating or bumping the trailing part of the string as needed)

http://www.cplusplus.com/reference/string/string/replace/

I wrote a quick test program to confirm my understanding, it seems like 
replacing short with long and long with short both work as expected:

https://gist.github.com/timarmstrong/b2e40308310d01faeb4dc1348baf4c58


http://gerrit.cloudera.org:8080/#/c/12120/6/be/src/util/cgroup-util.cc@133
PS6, Line 133:   *bytes = static_cast(min(v, 
numeric_limits::max()));
> Is this necessary? StringParse::StringToIntInternal caps the value at  std:
Ah yeah, I missed updating this while working on this code. StringToIntInternal 
handles it, but I need to treat PARSE_OVERFLOW as a non-error and also validate 
that it's not negative.


http://gerrit.cloudera.org:8080/#/c/12120/5/be/src/util/process-state-info.h
File be/src/util/process-state-info.h:

http://gerrit.cloudera.org:8080/#/c/12120/5/be/src/util/process-state-info.h@30
PS5, Line 30: Cgroup
> Nit: Remove?
Done. Thanks for catching



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 04 Jan 2019 00:41:25 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-03 Thread Tim Armstrong (Code Review)
Hello Pooja Nilangekar, Philip Zeyliger, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..

IMPALA-7941: part 1: detect cgroups memory limit

This adds the logic to detect the cgroups memory limit,
but does not use it to set the process memory limit yet.
The information obtained is logged at startup and accessible
through the Web UI.

The patch boils down to reading from the appropriate
places in the filesystem. The main complication is that paths need to be
translated to point to the right cgroup node inside the container.

This deletes some useless cgroup logic from ProcessStateInfo that
printed whatever happened to be the last cgroup in a file.

Testing:
Added a unit test to check that the code could parse the cgroups
hierarchy ok and return a positive value.

Tested on CentOS6 and CentOS7.

Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
---
M be/src/common/init.cc
M be/src/util/CMakeLists.txt
A be/src/util/cgroup-util.cc
A be/src/util/cgroup-util.h
M be/src/util/default-path-handlers.cc
M be/src/util/proc-info-test.cc
M be/src/util/process-state-info.cc
M be/src/util/process-state-info.h
M www/root.tmpl
9 files changed, 265 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/12120/7
--
To view, visit http://gerrit.cloudera.org:8080/12120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2019-01-03 Thread Pooja Nilangekar (Code Review)
Pooja Nilangekar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12120 )

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 6:

(4 comments)

Looks good. I just have a few questions.

http://gerrit.cloudera.org:8080/#/c/12120/6/be/src/util/cgroup-util.h
File be/src/util/cgroup-util.h:

http://gerrit.cloudera.org:8080/#/c/12120/6/be/src/util/cgroup-util.h@43
PS6, Line 43: is a
Nit: extra words?


http://gerrit.cloudera.org:8080/#/c/12120/6/be/src/util/cgroup-util.cc
File be/src/util/cgroup-util.cc:

http://gerrit.cloudera.org:8080/#/c/12120/6/be/src/util/cgroup-util.cc@111
PS6, Line 111:  paths.second.size(), paths.first
I am probably missing something here, would this cause an issue if the length 
of the relative path string is lesser than the length of the mount point 
string? Could we end up with an incomplete string?


http://gerrit.cloudera.org:8080/#/c/12120/6/be/src/util/cgroup-util.cc@133
PS6, Line 133:   *bytes = static_cast(min(v, 
numeric_limits::max()));
Is this necessary? StringParse::StringToIntInternal caps the value at  
std::numeric_limits::max()


http://gerrit.cloudera.org:8080/#/c/12120/5/be/src/util/process-state-info.h
File be/src/util/process-state-info.h:

http://gerrit.cloudera.org:8080/#/c/12120/5/be/src/util/process-state-info.h@30
PS5, Line 30: Cgroup
Nit: Remove?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Comment-Date: Thu, 03 Jan 2019 23:20:47 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2018-12-26 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12120 )

Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..


Patch Set 5:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Comment-Date: Wed, 26 Dec 2018 22:15:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit

2018-12-26 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12120


Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..

IMPALA-7941: part 1: detect cgroups memory limit

This adds the logic to detect the cgroups memory limit,
but does not use it to set the process memory limit yet.
The information obtained is logged at startup and accessible
through the Web UI.

The patch boils down to reading from the appropriate
places in the filesystem. The main complication is that paths need to be
translated to point to the right cgroup node inside the container.

This deletes some useless cgroup logic from ProcessStateInfo that
printed whatever happened to be the last cgroup in a file.

Testing:
Added a unit test to check that the code could parse the cgroups
hierarchy ok and return a positive value.

Tested on CentOS6 and CentOS7.

Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
---
M be/src/common/init.cc
M be/src/util/CMakeLists.txt
A be/src/util/cgroup-util.cc
A be/src/util/cgroup-util.h
M be/src/util/default-path-handlers.cc
M be/src/util/proc-info-test.cc
M be/src/util/process-state-info.cc
M be/src/util/process-state-info.h
M www/root.tmpl
9 files changed, 259 insertions(+), 50 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/12120/5
--
To view, visit http://gerrit.cloudera.org:8080/12120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins