[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15873375#comment-15873375 ] Yuliya Feldman commented on TWILL-216: -- Thank you guys for quick turnaround > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > Fix For: 0.10.0 > > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15873064#comment-15873064 ] ASF GitHub Bot commented on TWILL-216: -- Github user asfgit closed the pull request at: https://github.com/apache/twill/pull/33 > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > Fix For: 0.10.0 > > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872895#comment-15872895 ] ASF GitHub Bot commented on TWILL-216: -- Github user hsaputra commented on the issue: https://github.com/apache/twill/pull/33 Ah, Yulia just replied on JIRA that we HAD ALREADY used double for the ratio (not sure why) but this PR just continuing the existing data format. +1 then > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > Fix For: 0.10.0 > > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872892#comment-15872892 ] Henry Saputra commented on TWILL-216: - [~yufeldman] Gah, yeah you are right. I thought we had float for the initial ratio and change it to double. My mistake > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > Fix For: 0.10.0 > > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872884#comment-15872884 ] Yuliya Feldman commented on TWILL-216: -- [~hsaputra] What is your concern regarding double versus float? We already use double for that ratio. > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > Fix For: 0.10.0 > > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872882#comment-15872882 ] ASF GitHub Bot commented on TWILL-216: -- Github user hsaputra commented on the issue: https://github.com/apache/twill/pull/33 @chtyim I was asking concern about the selection of double vs float > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > Fix For: 0.10.0 > > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872876#comment-15872876 ] ASF GitHub Bot commented on TWILL-216: -- Github user chtyim commented on the issue: https://github.com/apache/twill/pull/33 LGTM. Will merge it when build pass. > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > Fix For: 0.10.0 > > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872281#comment-15872281 ] Yuliya Feldman commented on TWILL-216: -- Sorry, missed one style change. Will update PR in a minute > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872275#comment-15872275 ] Yuliya Feldman commented on TWILL-216: -- [~chtyim] Thank you for the reviews, I have updated PR with latest changes, also squashed commits > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872256#comment-15872256 ] ASF GitHub Bot commented on TWILL-216: -- Github user yufeldman commented on a diff in the pull request: https://github.com/apache/twill/pull/33#discussion_r101817212 --- Diff: twill-core/src/main/java/org/apache/twill/internal/json/TwillRuntimeSpecificationCodec.java --- @@ -94,6 +96,8 @@ public TwillRuntimeSpecification deserialize(JsonElement json, Type typeOfT, jsonObj.has(RM_SCHEDULER_ADDR) ? jsonObj.get(RM_SCHEDULER_ADDR).getAsString() : null, logLevels, - maxRetries); + maxRetries, + jsonObj.has(HEAP_RESERVED_MIN_RATIO) ? --- End diff -- sure > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872042#comment-15872042 ] ASF GitHub Bot commented on TWILL-216: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/33#discussion_r101789099 --- Diff: twill-core/src/main/java/org/apache/twill/internal/json/TwillRuntimeSpecificationCodec.java --- @@ -94,6 +96,8 @@ public TwillRuntimeSpecification deserialize(JsonElement json, Type typeOfT, jsonObj.has(RM_SCHEDULER_ADDR) ? jsonObj.get(RM_SCHEDULER_ADDR).getAsString() : null, logLevels, - maxRetries); + maxRetries, + jsonObj.has(HEAP_RESERVED_MIN_RATIO) ? --- End diff -- Should use HEAP_RESERVED_MIN_RATIO as the default instead of `null` > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872043#comment-15872043 ] ASF GitHub Bot commented on TWILL-216: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/33#discussion_r101789920 --- Diff: twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java --- @@ -38,7 +38,6 @@ import com.google.common.util.concurrent.SettableFuture; import com.google.gson.Gson; import com.google.gson.GsonBuilder; --- End diff -- Please don't update import spaces. If you use IntelliJ or eclipse, you can download the style from the twill site. > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15872041#comment-15872041 ] ASF GitHub Bot commented on TWILL-216: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/33#discussion_r101789313 --- Diff: twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java --- @@ -17,6 +17,8 @@ */ package org.apache.twill.yarn; +import joptsimple.OptionSpec; --- End diff -- Please don't reorder import. > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871441#comment-15871441 ] ASF GitHub Bot commented on TWILL-216: -- Github user hsaputra commented on a diff in the pull request: https://github.com/apache/twill/pull/33#discussion_r101708864 --- Diff: twill-core/src/main/java/org/apache/twill/internal/TwillContainerLauncher.java --- @@ -59,19 +59,21 @@ private final int instanceCount; private final JvmOptions jvmOpts; private final int reservedMemory; + private final Double heapToReservedRatio; private final Location secureStoreLocation; public TwillContainerLauncher(RuntimeSpecification runtimeSpec, ContainerInfo containerInfo, ProcessLauncher.PrepareLaunchContext launchContext, ZKClient zkClient, int instanceCount, JvmOptions jvmOpts, int reservedMemory, -Location secureStoreLocation) { +Location secureStoreLocation, Double heapToReservedRatio) { --- End diff -- do we need double here? Would float be ok? > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871362#comment-15871362 ] Yuliya Feldman commented on TWILL-216: -- [~chtyim] Absolutely - I'll try to address your comments ASAP > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871353#comment-15871353 ] Terence Yim commented on TWILL-216: --- [~yufeldman] I am ok with your approach. If you have time to address the comments by tomorrow, I will include the fix in the coming 0.10.0 release as well. > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871347#comment-15871347 ] ASF GitHub Bot commented on TWILL-216: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/33#discussion_r101699339 --- Diff: twill-core/src/main/java/org/apache/twill/internal/TwillContainerLauncher.java --- @@ -59,19 +59,21 @@ private final int instanceCount; private final JvmOptions jvmOpts; private final int reservedMemory; + private final Double heapToReservedRatio; private final Location secureStoreLocation; public TwillContainerLauncher(RuntimeSpecification runtimeSpec, ContainerInfo containerInfo, ProcessLauncher.PrepareLaunchContext launchContext, ZKClient zkClient, int instanceCount, JvmOptions jvmOpts, int reservedMemory, -Location secureStoreLocation) { +Location secureStoreLocation, Double heapToReservedRatio) { --- End diff -- Better to have `minHeapRatio` a `double` and the caller need to make sure it is not null. > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871350#comment-15871350 ] ASF GitHub Bot commented on TWILL-216: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/33#discussion_r101699007 --- Diff: twill-api/src/main/java/org/apache/twill/api/Configs.java --- @@ -34,6 +36,11 @@ public static final String JAVA_RESERVED_MEMORY_MB = "twill.java.reserved.memory.mb"; /** + * Configurable ratio between Heap and Reserved Memory + */ +public static final String HEAP_RESERVED_MIN_RATIO_CONFIG = "twill.java.reserved.memory.ratio"; --- End diff -- No need to have `_CONFIG` as it is already defined inside the `Keys` inner class. > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871351#comment-15871351 ] ASF GitHub Bot commented on TWILL-216: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/33#discussion_r101698978 --- Diff: twill-api/src/main/java/org/apache/twill/api/Configs.java --- @@ -75,6 +82,7 @@ private Keys() { */ public static final int JAVA_RESERVED_MEMORY_MB = 200; +public static final double HEAP_RESERVED_MIN_RATIO_DEFAULT = Constants.HEAP_MIN_RATIO; --- End diff -- Inside the `Defaults` class, no need to repeat the variable with `DEFAULT`. Just name it the same as the config key constant. > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871344#comment-15871344 ] ASF GitHub Bot commented on TWILL-216: -- Github user chtyim commented on the issue: https://github.com/apache/twill/pull/33 The approach seems fine. Just couple comments about the naming and not to allow `null` ratio. > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871345#comment-15871345 ] ASF GitHub Bot commented on TWILL-216: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/33#discussion_r101699381 --- Diff: twill-api/src/main/java/org/apache/twill/api/Configs.java --- @@ -75,6 +82,7 @@ private Keys() { */ public static final int JAVA_RESERVED_MEMORY_MB = 200; +public static final double HEAP_RESERVED_MIN_RATIO_DEFAULT = Constants.HEAP_MIN_RATIO; --- End diff -- You can remove the `Constants.HEAP_MIN_RATIO` and just define the default here. > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871348#comment-15871348 ] ASF GitHub Bot commented on TWILL-216: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/33#discussion_r101699525 --- Diff: twill-core/src/main/java/org/apache/twill/internal/json/TwillRuntimeSpecificationCodec.java --- @@ -45,6 +45,7 @@ private static final String TWILL_RUNID = "twillRunId"; private static final String TWILL_APP_NAME = "twillAppName"; private static final String RESERVED_MEMORY = "reservedMemory"; + private static final String HEAP_TO_RESERVED_RATIO = "heapToReservedRatio"; --- End diff -- for consistency, just have everywhere call it `minHeapRatio` > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871349#comment-15871349 ] ASF GitHub Bot commented on TWILL-216: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/33#discussion_r101699224 --- Diff: twill-api/src/main/java/org/apache/twill/api/Configs.java --- @@ -34,6 +36,11 @@ public static final String JAVA_RESERVED_MEMORY_MB = "twill.java.reserved.memory.mb"; /** + * Configurable ratio between Heap and Reserved Memory + */ +public static final String HEAP_RESERVED_MIN_RATIO_CONFIG = "twill.java.reserved.memory.ratio"; --- End diff -- Also, I think the config key should be `twill.java.min.heap.memory.ratio`, as it is the minimum ratio of heap memory. > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871346#comment-15871346 ] ASF GitHub Bot commented on TWILL-216: -- Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/33#discussion_r101699432 --- Diff: twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java --- @@ -41,11 +41,23 @@ private final String rmSchedulerAddr; private final Map> logLevels; private final Map maxRetries; + private Double heapToReserveRatio; --- End diff -- Shouldn't allow null. This is an internal class, you can simply require the caller to pass in valid value. > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871248#comment-15871248 ] Yuliya Feldman commented on TWILL-216: -- Sorry for not adding more details soon. Essentially at the moment Twill decides how much of the total requested memory to allocate to heap based on the hardcoded ratio of 0.7, meaning it will allocate at least 70% to heap. There could be applications that use direct memory quite a bit and they want to allocate more then 30% of total memory to be direct memory. This is a rational behind this JIRA. > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15871071#comment-15871071 ] ASF GitHub Bot commented on TWILL-216: -- Github user hsaputra commented on the issue: https://github.com/apache/twill/pull/33 @yufeldman : Could you kindly add in the summary of the PR the summary of the approach you are taking to fix the issue? Will help with the review to compare expectation vs implementation. > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TWILL-216) Make ratio between total memory and on-heap memory configurable
[ https://issues.apache.org/jira/browse/TWILL-216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15870863#comment-15870863 ] ASF GitHub Bot commented on TWILL-216: -- GitHub user yufeldman opened a pull request: https://github.com/apache/twill/pull/33 TWILL-216 Make ratio between total memory and on-heap memory configur… …able You can merge this pull request into a Git repository by running: $ git pull https://github.com/yufeldman/twill branch-TWILL-216 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/twill/pull/33.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #33 commit 6f721f696452bb6124cf5b53b285fe60db75ff03 Author: Yuliya Feldman Date: 2017-02-10T21:05:20Z TWILL-216 Make ratio between total memory and on-heap memory configurable > Make ratio between total memory and on-heap memory configurable > --- > > Key: TWILL-216 > URL: https://issues.apache.org/jira/browse/TWILL-216 > Project: Apache Twill > Issue Type: Improvement > Components: yarn >Reporter: Yuliya Feldman >Assignee: Yuliya Feldman > > As of now ratio between on-heap memory and total memory provided to yarn > container is hardcoded to 0.7, so if app running in the container needs more > reserved memory than on-heap it is not possible to achieve. > Suggestion is to make it configurable as well as amount of reserved memory -- This message was sent by Atlassian JIRA (v6.3.15#6346)