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

["Pooja Nilangekar (Code Review)" <ge...@cloudera.org>]

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<int64_t>(min<size_t>(v, numeric_limits<int64_t>::max()));
Is this necessary? StringParse::StringToIntInternal caps the value at  std::numeric_limits<T>::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 <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Thu, 03 Jan 2019 23:20:47 +0000
Gerrit-HasComments: Yes