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 <tarmstr...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <pooja.nilange...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jan 2019 00:45:40 +0000
Gerrit-HasComments: Yes

Reply via email to