Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12262 )

Change subject: IMPALA-7941: part 2/2: use cgroups memory limit
......................................................................


Patch Set 3: Code-Review+1

(2 comments)

Looks good to me. I talked to Bikram, and he's going to finish the review.

http://gerrit.cloudera.org:8080/#/c/12262/2/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

http://gerrit.cloudera.org:8080/#/c/12262/2/be/src/runtime/exec-env.cc@364
PS2, Line 364:   if (use_commit_limit) {
             :     avail_mem = MemInfo::commit_limit();
> I pulled out this logic into a function since it was getting unwieldy and a
Makes sense, that is much cleaner.


http://gerrit.cloudera.org:8080/#/c/12262/2/docker/daemon_entrypoint.sh
File docker/daemon_entrypoint.sh:

http://gerrit.cloudera.org:8080/#/c/12262/2/docker/daemon_entrypoint.sh@45
PS2, Line 45: export JAVA_TOOL_OPTIONS="-Xmx2g $JAVA_TOOL_OPTIONS"
> I did see that, but I didn't want to embed test-specific logic into the con
Yeah, I figured that this was already on our TODO list somewhere.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9fb4fb936a46fc194a204391d03c07c8c7fba21
Gerrit-Change-Number: 12262
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Fri, 25 Jan 2019 23:08:27 +0000
Gerrit-HasComments: Yes

Reply via email to