[GitHub] twill pull request #33: TWILL-216 Make ratio between total memory and on-hea...
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?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[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)
[GitHub] twill pull request #33: TWILL-216 Make ratio between total memory and on-hea...
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. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] twill pull request #33: TWILL-216 Make ratio between total memory and on-hea...
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` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] twill pull request #33: TWILL-216 Make ratio between total memory and on-hea...
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. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[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=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=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)
[GitHub] twill pull request #33: TWILL-216 Make ratio between total memory and on-hea...
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 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[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=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=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)
[GitHub] twill issue #33: TWILL-216 Make ratio between total memory and on-heap memor...
Github user chtyim commented on the issue: https://github.com/apache/twill/pull/33 LGTM. Will merge it when build pass. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[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] [Updated] (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:all-tabpanel ] Terence Yim updated TWILL-216: -- Fix Version/s: 0.10.0 > 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)
[GitHub] twill issue #33: TWILL-216 Make ratio between total memory and on-heap memor...
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 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[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=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)
[GitHub] twill issue #33: TWILL-216 Make ratio between total memory and on-heap memor...
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 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[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=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)
