You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@nemo.apache.org by GitBox <gi...@apache.org> on 2020/01/14 07:32:19 UTC
[GitHub] [incubator-nemo] polarcoke2 opened a new pull request #279: Hwarim
maxoffheap
polarcoke2 opened a new pull request #279: Hwarim maxoffheap
URL: https://github.com/apache/incubator-nemo/pull/279
JIRA: [NEMO-420: TITLE](https://issues.apache.org/jira/projects/NEMO/issues/NEMO-420)
**Major changes:**
- enabled OffHeapMemory configuration for multiple types of executors
**Minor changes to note:**
- changed default MaxOffHeapRatio to 0.02 from 0.2
- modified typos (poison from posion)
**Tests for the changes:**
- N/A
**Other comments:**
- N/A
Closes #GITHUB_PR_NUMBER
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] wonook commented on a change in pull request #279:
[NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
wonook commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366946672
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/resource/ContainerManager.java
##########
@@ -149,18 +149,39 @@ public void onContainerAllocated(final String executorId,
}
final ResourceSpecification resourceSpecification = selectResourceSpecForContainer();
+ final List<Configuration> configurationsToMerge = new ArrayList<>();
+
evaluatorIdToResourceSpec.put(allocatedContainer.getId(), resourceSpecification);
LOG.info("Container type (" + resourceSpecification.getContainerType()
+ ") allocated, will be used for [" + executorId + "]");
pendingContextIdToResourceSpec.put(executorId, resourceSpecification);
+ configurationsToMerge.add(executorConfiguration);
+
+ // ExecutorMemory handling
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.ExecutorMemoryMb.class, String.valueOf(resourceSpecification.getMemory()))
+ .build()
+ );
+
+ // MaxOffheapRatio handling
+ resourceSpecification.getMaxOffheapRatio().ifPresent(value -> {
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.MaxOffheapRatio.class, String.valueOf(resourceSpecification.getMaxOffheapRatio()))
Review comment:
I think `resourceSpecification.getMaxOffheapRatio()` should become `value`, as that is the actual value of the parameter.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] wonook commented on a change in pull request #279:
[NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
wonook commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366945015
##########
File path: runtime/executor/src/main/java/org/apache/nemo/runtime/executor/data/MemoryPoolAssigner.java
##########
@@ -47,18 +47,18 @@
private final MemoryPool memoryPool;
@Inject
- public MemoryPoolAssigner(@Parameter(JobConf.MaxOffheapMb.class) final int maxOffheapMb,
+ public MemoryPoolAssigner(@Parameter(JobConf.ExecutorMemoryMb.class) final int memory,
+ @Parameter(JobConf.MaxOffheapRatio.class) final double maxOffheapRatio,
@Parameter(JobConf.ChunkSizeKb.class) final int chunkSizeKb) {
+
+ int maxOffheapMb = (int) (memory * maxOffheapRatio);
if (chunkSizeKb < MIN_CHUNK_SIZE_KB) {
throw new IllegalArgumentException("Chunk size too small. Minimum chunk size is 4KB");
}
final long maxNumChunks = (long) maxOffheapMb * 1024 / chunkSizeKb;
if (maxNumChunks > Integer.MAX_VALUE) {
throw new IllegalArgumentException("Too many pages to allocate (exceeds MAX_INT)");
}
- if (maxNumChunks < 1) {
- throw new IllegalArgumentException("The given amount of memory amounted to less than one chunk.");
- }
Review comment:
Let's put this back in, as we can assume that the max off heap ratio is not 0 at all times.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] polarcoke2 commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
polarcoke2 commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367235661
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/resource/ContainerManager.java
##########
@@ -149,18 +149,39 @@ public void onContainerAllocated(final String executorId,
}
final ResourceSpecification resourceSpecification = selectResourceSpecForContainer();
+ final List<Configuration> configurationsToMerge = new ArrayList<>();
+
evaluatorIdToResourceSpec.put(allocatedContainer.getId(), resourceSpecification);
LOG.info("Container type (" + resourceSpecification.getContainerType()
+ ") allocated, will be used for [" + executorId + "]");
pendingContextIdToResourceSpec.put(executorId, resourceSpecification);
+ configurationsToMerge.add(executorConfiguration);
+
+ // ExecutorMemory handling
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.ExecutorMemoryMb.class, String.valueOf(resourceSpecification.getMemory()))
+ .build()
+ );
+
+ // MaxOffheapRatio handling
+ resourceSpecification.getMaxOffheapRatio().ifPresent(value -> {
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.MaxOffheapRatio.class, String.valueOf(value))
+ .build()
+ );
+ });
+
// Poison handling
- final Configuration poisonConfiguration = Tang.Factory.getTang().newConfigurationBuilder()
- .bindNamedParameter(JobConf.ExecutorPosionSec.class, String.valueOf(resourceSpecification.getPoisonSec()))
- .build();
+ resourceSpecification.getPoisonSec().ifPresent(value -> {
Review comment:
good point, thank you.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] DifferentSC commented on issue #279: [NEMO-420]
OffHeapMemory configuration only supports a single type of executor
Posted by GitBox <gi...@apache.org>.
DifferentSC commented on issue #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#issuecomment-574970572
@wonook Please look at this PR and merge if it is okay for you?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] polarcoke2 commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
polarcoke2 commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367197094
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/resource/ContainerManager.java
##########
@@ -149,18 +149,39 @@ public void onContainerAllocated(final String executorId,
}
final ResourceSpecification resourceSpecification = selectResourceSpecForContainer();
+ final List<Configuration> configurationsToMerge = new ArrayList<>();
+
evaluatorIdToResourceSpec.put(allocatedContainer.getId(), resourceSpecification);
LOG.info("Container type (" + resourceSpecification.getContainerType()
+ ") allocated, will be used for [" + executorId + "]");
pendingContextIdToResourceSpec.put(executorId, resourceSpecification);
+ configurationsToMerge.add(executorConfiguration);
+
+ // ExecutorMemory handling
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.ExecutorMemoryMb.class, String.valueOf(resourceSpecification.getMemory()))
+ .build()
+ );
+
+ // MaxOffheapRatio handling
+ resourceSpecification.getMaxOffheapRatio().ifPresent(value -> {
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.MaxOffheapRatio.class, String.valueOf(resourceSpecification.getMaxOffheapRatio()))
+ .build()
+ );
+ });
+
// Poison handling
- final Configuration poisonConfiguration = Tang.Factory.getTang().newConfigurationBuilder()
- .bindNamedParameter(JobConf.ExecutorPosionSec.class, String.valueOf(resourceSpecification.getPoisonSec()))
- .build();
+ resourceSpecification.getPoisonSec().ifPresent(value -> {
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.ExecutorPoisonSec.class, String.valueOf(resourceSpecification.getPoisonSec()))
Review comment:
Also changed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] polarcoke2 commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
polarcoke2 commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367197094
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/resource/ContainerManager.java
##########
@@ -149,18 +149,39 @@ public void onContainerAllocated(final String executorId,
}
final ResourceSpecification resourceSpecification = selectResourceSpecForContainer();
+ final List<Configuration> configurationsToMerge = new ArrayList<>();
+
evaluatorIdToResourceSpec.put(allocatedContainer.getId(), resourceSpecification);
LOG.info("Container type (" + resourceSpecification.getContainerType()
+ ") allocated, will be used for [" + executorId + "]");
pendingContextIdToResourceSpec.put(executorId, resourceSpecification);
+ configurationsToMerge.add(executorConfiguration);
+
+ // ExecutorMemory handling
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.ExecutorMemoryMb.class, String.valueOf(resourceSpecification.getMemory()))
+ .build()
+ );
+
+ // MaxOffheapRatio handling
+ resourceSpecification.getMaxOffheapRatio().ifPresent(value -> {
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.MaxOffheapRatio.class, String.valueOf(resourceSpecification.getMaxOffheapRatio()))
+ .build()
+ );
+ });
+
// Poison handling
- final Configuration poisonConfiguration = Tang.Factory.getTang().newConfigurationBuilder()
- .bindNamedParameter(JobConf.ExecutorPosionSec.class, String.valueOf(resourceSpecification.getPoisonSec()))
- .build();
+ resourceSpecification.getPoisonSec().ifPresent(value -> {
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.ExecutorPoisonSec.class, String.valueOf(resourceSpecification.getPoisonSec()))
Review comment:
Also changed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] polarcoke2 commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
polarcoke2 commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366786021
##########
File path: conf/src/main/java/org/apache/nemo/conf/JobConf.java
##########
@@ -332,6 +316,22 @@
public final class ExecutorId implements Name<String> {
}
+ /**
+ * Maximum off-heap memory ratio to the total memory in the executor.
+ */
+ @NamedParameter(doc = "The maximum ratio of off-heap memory size to the total memory size.",
+ short_name = "max_offheap_ratio", default_value = "0.02")
Review comment:
thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] polarcoke2 commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
polarcoke2 commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367196545
##########
File path: runtime/executor/src/main/java/org/apache/nemo/runtime/executor/data/MemoryPoolAssigner.java
##########
@@ -47,18 +47,18 @@
private final MemoryPool memoryPool;
@Inject
- public MemoryPoolAssigner(@Parameter(JobConf.MaxOffheapMb.class) final int maxOffheapMb,
+ public MemoryPoolAssigner(@Parameter(JobConf.ExecutorMemoryMb.class) final int memory,
+ @Parameter(JobConf.MaxOffheapRatio.class) final double maxOffheapRatio,
@Parameter(JobConf.ChunkSizeKb.class) final int chunkSizeKb) {
+
+ int maxOffheapMb = (int) (memory * maxOffheapRatio);
if (chunkSizeKb < MIN_CHUNK_SIZE_KB) {
throw new IllegalArgumentException("Chunk size too small. Minimum chunk size is 4KB");
}
final long maxNumChunks = (long) maxOffheapMb * 1024 / chunkSizeKb;
if (maxNumChunks > Integer.MAX_VALUE) {
throw new IllegalArgumentException("Too many pages to allocate (exceeds MAX_INT)");
}
- if (maxNumChunks < 1) {
- throw new IllegalArgumentException("The given amount of memory amounted to less than one chunk.");
- }
Review comment:
I've added this condition, thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] polarcoke2 commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
polarcoke2 commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367235694
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/resource/ContainerManager.java
##########
@@ -149,18 +149,39 @@ public void onContainerAllocated(final String executorId,
}
final ResourceSpecification resourceSpecification = selectResourceSpecForContainer();
+ final List<Configuration> configurationsToMerge = new ArrayList<>();
+
evaluatorIdToResourceSpec.put(allocatedContainer.getId(), resourceSpecification);
LOG.info("Container type (" + resourceSpecification.getContainerType()
+ ") allocated, will be used for [" + executorId + "]");
pendingContextIdToResourceSpec.put(executorId, resourceSpecification);
+ configurationsToMerge.add(executorConfiguration);
+
+ // ExecutorMemory handling
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.ExecutorMemoryMb.class, String.valueOf(resourceSpecification.getMemory()))
+ .build()
+ );
+
+ // MaxOffheapRatio handling
+ resourceSpecification.getMaxOffheapRatio().ifPresent(value -> {
Review comment:
same here
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] wonook merged pull request #279: [NEMO-420]
OffHeapMemory configuration only supports a single type of executor
Posted by GitBox <gi...@apache.org>.
wonook merged pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] wonook commented on a change in pull request #279:
[NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
wonook commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367233546
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/resource/ResourceSpecification.java
##########
@@ -29,22 +31,25 @@
private final String containerType;
private final int capacity;
private final int memory;
- private final int poisonSec; // -1 if this resources is not poisoned
+ private final Optional<Double> maxOffheapRatio;
+ private final Optional<Integer> poisonSec; // -1 if this resources is not poisoned
Review comment:
Let's change this to `OptionalDouble` and `OptionalInt` which provides more features for the specific data types
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] wonook commented on a change in pull request #279:
[NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
wonook commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366946832
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/resource/ContainerManager.java
##########
@@ -149,18 +149,39 @@ public void onContainerAllocated(final String executorId,
}
final ResourceSpecification resourceSpecification = selectResourceSpecForContainer();
+ final List<Configuration> configurationsToMerge = new ArrayList<>();
+
evaluatorIdToResourceSpec.put(allocatedContainer.getId(), resourceSpecification);
LOG.info("Container type (" + resourceSpecification.getContainerType()
+ ") allocated, will be used for [" + executorId + "]");
pendingContextIdToResourceSpec.put(executorId, resourceSpecification);
+ configurationsToMerge.add(executorConfiguration);
+
+ // ExecutorMemory handling
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.ExecutorMemoryMb.class, String.valueOf(resourceSpecification.getMemory()))
+ .build()
+ );
+
+ // MaxOffheapRatio handling
+ resourceSpecification.getMaxOffheapRatio().ifPresent(value -> {
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.MaxOffheapRatio.class, String.valueOf(resourceSpecification.getMaxOffheapRatio()))
+ .build()
+ );
+ });
+
// Poison handling
- final Configuration poisonConfiguration = Tang.Factory.getTang().newConfigurationBuilder()
- .bindNamedParameter(JobConf.ExecutorPosionSec.class, String.valueOf(resourceSpecification.getPoisonSec()))
- .build();
+ resourceSpecification.getPoisonSec().ifPresent(value -> {
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.ExecutorPoisonSec.class, String.valueOf(resourceSpecification.getPoisonSec()))
Review comment:
Same here, `value` instead of `...getPoisonSec()`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] wonook commented on a change in pull request #279:
[NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
wonook commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366187205
##########
File path: conf/src/main/java/org/apache/nemo/conf/JobConf.java
##########
@@ -332,6 +316,22 @@
public final class ExecutorId implements Name<String> {
}
+ /**
+ * Maximum off-heap memory ratio to the total memory in the executor.
+ */
+ @NamedParameter(doc = "The maximum ratio of off-heap memory size to the total memory size.",
+ short_name = "max_offheap_ratio", default_value = "0.02")
Review comment:
I think the `default_value` here doesn't make too much sense, as we set the default value in `RuntimeMaster:319`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] DifferentSC commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
DifferentSC commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366189672
##########
File path: examples/pom.xml
##########
@@ -32,7 +32,7 @@ under the License.
<modules>
<module>beam</module>
- <module>spark</module>
+<!-- <module>spark</module>-->
Review comment:
I guess this change is irrelevant to the PR topic.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] DifferentSC commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
DifferentSC commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366190257
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/RuntimeMaster.java
##########
@@ -313,11 +314,15 @@ public void requestContainer(final String resourceSpecificationString) {
final TreeNode resourceNode = jsonRootNode.get(i);
final String type = resourceNode.get("type").traverse().nextTextValue();
final int memory = resourceNode.get("memory_mb").traverse().getIntValue();
+ final double maxOffheapRatio =
+ (resourceNode.path("max_offheap_ratio").traverse().nextToken() == JsonToken.VALUE_NUMBER_FLOAT)
+ ? resourceNode.path("max_offheap_ratio").traverse().getFloatValue() : 0.02;
Review comment:
Same here too. I see no good rationales on setting the default value to `0.02`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] DifferentSC commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
DifferentSC commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367203843
##########
File path: runtime/executor/src/test/java/org/apache/nemo/runtime/executor/data/BlockStoreTest.java
##########
@@ -229,7 +229,7 @@ public void setUp() throws Exception {
public void testMemoryStore() throws Exception {
final Injector injector = Tang.Factory.getTang().newInjector();
injector.bindVolatileInstance(SerializerManager.class, serializerManager);
- injector.bindVolatileParameter(JobConf.MaxOffheapMb.class, 128);
+ injector.bindVolatileParameter(JobConf.ExecutorMemoryMb.class, 640);
Review comment:
I think it would be more readable if we specify the `MaxOffheapRatio` to `0.2`. Others may not be aware of the default value.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] DifferentSC commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
DifferentSC commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367204033
##########
File path: runtime/executor/src/test/java/org/apache/nemo/runtime/executor/datatransfer/DataTransferTest.java
##########
@@ -183,7 +183,7 @@ public void tearDown() throws IOException {
final Configuration executorConfiguration = TANG.newConfigurationBuilder()
.bindNamedParameter(JobConf.ExecutorId.class, executorId)
.bindNamedParameter(MessageParameters.SenderId.class, executorId)
- .bindNamedParameter(JobConf.MaxOffheapMb.class, "128")
+ .bindNamedParameter(JobConf.ExecutorMemoryMb.class, "640")
Review comment:
Same here too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] DifferentSC commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
DifferentSC commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367203905
##########
File path: runtime/executor/src/test/java/org/apache/nemo/runtime/executor/data/BlockStoreTest.java
##########
@@ -263,7 +263,7 @@ public void testLocalFileStore() throws Exception {
final Injector injector = Tang.Factory.getTang().newInjector();
injector.bindVolatileParameter(JobConf.FileDirectory.class, TMP_FILE_DIRECTORY);
injector.bindVolatileInstance(SerializerManager.class, serializerManager);
- injector.bindVolatileParameter(JobConf.MaxOffheapMb.class, 128);
+ injector.bindVolatileParameter(JobConf.ExecutorMemoryMb.class, 640);
Review comment:
Same here too
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] DifferentSC commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
DifferentSC commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366189445
##########
File path: conf/src/main/java/org/apache/nemo/conf/JobConf.java
##########
@@ -332,6 +316,22 @@
public final class ExecutorId implements Name<String> {
}
+ /**
+ * Maximum off-heap memory ratio to the total memory in the executor.
+ */
+ @NamedParameter(doc = "The maximum ratio of off-heap memory size to the total memory size.",
+ short_name = "max_offheap_ratio", default_value = "0.02")
Review comment:
Why `0.02`? `0.02` is smaller than the previous default value `0.2`, but it is basically same in that some portion of executor on-heap memory is allocated in an unnoticeable way. I think we'd better make it to `0`, and enable off-heap memory management only when this parameter becomes bigger than 0.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] DifferentSC commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
DifferentSC commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367203876
##########
File path: runtime/executor/src/test/java/org/apache/nemo/runtime/executor/data/BlockStoreTest.java
##########
@@ -245,7 +245,7 @@ public void testMemoryStore() throws Exception {
public void testSerMemoryStore() throws Exception {
final Injector injector = Tang.Factory.getTang().newInjector();
injector.bindVolatileInstance(SerializerManager.class, serializerManager);
- injector.bindVolatileParameter(JobConf.MaxOffheapMb.class, 128);
+ injector.bindVolatileParameter(JobConf.ExecutorMemoryMb.class, 640);
Review comment:
Same here too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] polarcoke2 commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
polarcoke2 commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367196470
##########
File path: conf/src/main/java/org/apache/nemo/conf/JobConf.java
##########
@@ -332,6 +316,31 @@
public final class ExecutorId implements Name<String> {
}
+ /**
+ * Maximum off-heap memory ratio to the total memory in the executor.
+ */
+ @NamedParameter(doc = "The maximum ratio of off-heap memory size to the total memory size.",
+ short_name = "max_offheap_ratio", default_value = "0.2")
+ public final class MaxOffheapRatio implements Name<Double> {
+ }
+
+ /**
+ * Maximum off-heap memory size in the executor.
+ * This is set by the system according to the off-heap ratio.
+ */
+ //@NamedParameter(doc = "The maximum off-heap memory that can be allocated")
+ //public final class MaxOffheapMb implements Name<Integer> {
+ //}
Review comment:
I've deleted this configuration and changed the order of other configurations in an appropriate way, also added short name for ExecutorMemoryMb as "memory_mb", as we look for memory in runtime master (function request container) with this string.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] wonook commented on a change in pull request #279:
[NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
wonook commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366187387
##########
File path: examples/pom.xml
##########
@@ -32,7 +32,7 @@ under the License.
<modules>
<module>beam</module>
- <module>spark</module>
+<!-- <module>spark</module>-->
Review comment:
I think this change must have gotten in there by mistake, let's revert this change.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] wonook commented on a change in pull request #279:
[NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
wonook commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367232916
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/resource/ContainerManager.java
##########
@@ -149,18 +149,39 @@ public void onContainerAllocated(final String executorId,
}
final ResourceSpecification resourceSpecification = selectResourceSpecForContainer();
+ final List<Configuration> configurationsToMerge = new ArrayList<>();
+
evaluatorIdToResourceSpec.put(allocatedContainer.getId(), resourceSpecification);
LOG.info("Container type (" + resourceSpecification.getContainerType()
+ ") allocated, will be used for [" + executorId + "]");
pendingContextIdToResourceSpec.put(executorId, resourceSpecification);
+ configurationsToMerge.add(executorConfiguration);
+
+ // ExecutorMemory handling
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.ExecutorMemoryMb.class, String.valueOf(resourceSpecification.getMemory()))
+ .build()
+ );
+
+ // MaxOffheapRatio handling
+ resourceSpecification.getMaxOffheapRatio().ifPresent(value -> {
Review comment:
Let's get rid of the { and }, and ;, as it causes a sonar cloud issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] DifferentSC commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
DifferentSC commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366191281
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/resource/ResourceSpecification.java
##########
@@ -29,22 +29,34 @@
private final String containerType;
private final int capacity;
private final int memory;
+ private final double maxOffheapRatio;
+ private final int maxOffheapMb;
private final int poisonSec; // -1 if this resources is not poisoned
public ResourceSpecification(final String containerType,
final int capacity,
final int memory) {
- this(containerType, capacity, memory, -1);
+ this(containerType, capacity, memory, 0.02, -1);
Review comment:
Same here for `0.02`. In addition, I don't think it's good to scatter the same default value `0.02` to many places.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] polarcoke2 commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
polarcoke2 commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366785942
##########
File path: conf/src/main/java/org/apache/nemo/conf/JobConf.java
##########
@@ -332,6 +316,22 @@
public final class ExecutorId implements Name<String> {
}
+ /**
+ * Maximum off-heap memory ratio to the total memory in the executor.
+ */
+ @NamedParameter(doc = "The maximum ratio of off-heap memory size to the total memory size.",
+ short_name = "max_offheap_ratio", default_value = "0.02")
Review comment:
refactored
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] wonook commented on a change in pull request #279:
[NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
wonook commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367232890
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/resource/ContainerManager.java
##########
@@ -149,18 +149,39 @@ public void onContainerAllocated(final String executorId,
}
final ResourceSpecification resourceSpecification = selectResourceSpecForContainer();
+ final List<Configuration> configurationsToMerge = new ArrayList<>();
+
evaluatorIdToResourceSpec.put(allocatedContainer.getId(), resourceSpecification);
LOG.info("Container type (" + resourceSpecification.getContainerType()
+ ") allocated, will be used for [" + executorId + "]");
pendingContextIdToResourceSpec.put(executorId, resourceSpecification);
+ configurationsToMerge.add(executorConfiguration);
+
+ // ExecutorMemory handling
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.ExecutorMemoryMb.class, String.valueOf(resourceSpecification.getMemory()))
+ .build()
+ );
+
+ // MaxOffheapRatio handling
+ resourceSpecification.getMaxOffheapRatio().ifPresent(value -> {
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.MaxOffheapRatio.class, String.valueOf(value))
+ .build()
+ );
+ });
+
// Poison handling
- final Configuration poisonConfiguration = Tang.Factory.getTang().newConfigurationBuilder()
- .bindNamedParameter(JobConf.ExecutorPosionSec.class, String.valueOf(resourceSpecification.getPoisonSec()))
- .build();
+ resourceSpecification.getPoisonSec().ifPresent(value -> {
Review comment:
Let's get rid of the `{` and `}`, and `;`, as it causes a sonar cloud issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] wonook merged pull request #279: [NEMO-420]
OffHeapMemory configuration only supports a single type of executor
Posted by GitBox <gi...@apache.org>.
wonook merged pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] polarcoke2 commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
polarcoke2 commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366783750
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/resource/ResourceSpecification.java
##########
@@ -29,22 +29,34 @@
private final String containerType;
private final int capacity;
private final int memory;
+ private final double maxOffheapRatio;
+ private final int maxOffheapMb;
private final int poisonSec; // -1 if this resources is not poisoned
public ResourceSpecification(final String containerType,
final int capacity,
final int memory) {
- this(containerType, capacity, memory, -1);
+ this(containerType, capacity, memory, 0.02, -1);
}
public ResourceSpecification(final String containerType,
Review comment:
I removed the constructor. Thanks for your comment.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] polarcoke2 commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
polarcoke2 commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366784953
##########
File path: conf/src/main/java/org/apache/nemo/conf/JobConf.java
##########
@@ -332,6 +316,22 @@
public final class ExecutorId implements Name<String> {
}
+ /**
+ * Maximum off-heap memory ratio to the total memory in the executor.
+ */
+ @NamedParameter(doc = "The maximum ratio of off-heap memory size to the total memory size.",
+ short_name = "max_offheap_ratio", default_value = "0.02")
Review comment:
changed default value to 0.2, please check.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] polarcoke2 commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
polarcoke2 commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366701623
##########
File path: examples/pom.xml
##########
@@ -32,7 +32,7 @@ under the License.
<modules>
<module>beam</module>
- <module>spark</module>
+<!-- <module>spark</module>-->
Review comment:
Changed. Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] wonook commented on a change in pull request #279:
[NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
wonook commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366944159
##########
File path: conf/src/main/java/org/apache/nemo/conf/JobConf.java
##########
@@ -332,6 +316,31 @@
public final class ExecutorId implements Name<String> {
}
+ /**
+ * Maximum off-heap memory ratio to the total memory in the executor.
+ */
+ @NamedParameter(doc = "The maximum ratio of off-heap memory size to the total memory size.",
+ short_name = "max_offheap_ratio", default_value = "0.2")
+ public final class MaxOffheapRatio implements Name<Double> {
+ }
+
+ /**
+ * Maximum off-heap memory size in the executor.
+ * This is set by the system according to the off-heap ratio.
+ */
+ //@NamedParameter(doc = "The maximum off-heap memory that can be allocated")
+ //public final class MaxOffheapMb implements Name<Integer> {
+ //}
Review comment:
Let's get rid of the unused configuration
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] DifferentSC commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
DifferentSC commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367203927
##########
File path: runtime/executor/src/test/java/org/apache/nemo/runtime/executor/data/BlockStoreTest.java
##########
@@ -300,7 +300,7 @@ private GlusterFileStore createGlusterFileStore(final String executorId)
injector.bindVolatileParameter(JobConf.JobId.class, "GFS test");
injector.bindVolatileParameter(JobConf.ExecutorId.class, executorId);
injector.bindVolatileInstance(SerializerManager.class, serializerManager);
- injector.bindVolatileParameter(JobConf.MaxOffheapMb.class, 128);
+ injector.bindVolatileParameter(JobConf.ExecutorMemoryMb.class, 640);
Review comment:
Same here too
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] polarcoke2 commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
polarcoke2 commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367213542
##########
File path: runtime/executor/src/test/java/org/apache/nemo/runtime/executor/data/BlockStoreTest.java
##########
@@ -229,7 +229,7 @@ public void setUp() throws Exception {
public void testMemoryStore() throws Exception {
final Injector injector = Tang.Factory.getTang().newInjector();
injector.bindVolatileInstance(SerializerManager.class, serializerManager);
- injector.bindVolatileParameter(JobConf.MaxOffheapMb.class, 128);
+ injector.bindVolatileParameter(JobConf.ExecutorMemoryMb.class, 640);
Review comment:
I have explicitly binded MaxOffheapRatio with default value 0.2.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] DifferentSC commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
DifferentSC commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366190453
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/RuntimeMaster.java
##########
@@ -313,11 +314,15 @@ public void requestContainer(final String resourceSpecificationString) {
final TreeNode resourceNode = jsonRootNode.get(i);
final String type = resourceNode.get("type").traverse().nextTextValue();
final int memory = resourceNode.get("memory_mb").traverse().getIntValue();
+ final double maxOffheapRatio =
+ (resourceNode.path("max_offheap_ratio").traverse().nextToken() == JsonToken.VALUE_NUMBER_FLOAT)
+ ? resourceNode.path("max_offheap_ratio").traverse().getFloatValue() : 0.02;
Review comment:
If this value is not explicitly set, I think it'd be good to set this value to `0`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] wonook commented on a change in pull request #279:
[NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
wonook commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366187914
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/resource/ResourceSpecification.java
##########
@@ -29,22 +29,34 @@
private final String containerType;
private final int capacity;
private final int memory;
+ private final double maxOffheapRatio;
+ private final int maxOffheapMb;
private final int poisonSec; // -1 if this resources is not poisoned
public ResourceSpecification(final String containerType,
final int capacity,
final int memory) {
- this(containerType, capacity, memory, -1);
+ this(containerType, capacity, memory, 0.02, -1);
}
public ResourceSpecification(final String containerType,
Review comment:
I think this constructor wouldn't be needed, as the constructor above is only available for testing. Let's remove this unused constructor.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] polarcoke2 commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
polarcoke2 commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367235793
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/resource/ResourceSpecification.java
##########
@@ -29,22 +31,25 @@
private final String containerType;
private final int capacity;
private final int memory;
- private final int poisonSec; // -1 if this resources is not poisoned
+ private final Optional<Double> maxOffheapRatio;
+ private final Optional<Integer> poisonSec; // -1 if this resources is not poisoned
Review comment:
done! thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] polarcoke2 commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
polarcoke2 commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r366783256
##########
File path: examples/pom.xml
##########
@@ -32,7 +32,7 @@ under the License.
<modules>
<module>beam</module>
- <module>spark</module>
+<!-- <module>spark</module>-->
Review comment:
changed! thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nemo] polarcoke2 commented on a change in pull request
#279: [NEMO-420] OffHeapMemory configuration only supports a single type of
executor
Posted by GitBox <gi...@apache.org>.
polarcoke2 commented on a change in pull request #279: [NEMO-420] OffHeapMemory configuration only supports a single type of executor
URL: https://github.com/apache/incubator-nemo/pull/279#discussion_r367197182
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/resource/ContainerManager.java
##########
@@ -149,18 +149,39 @@ public void onContainerAllocated(final String executorId,
}
final ResourceSpecification resourceSpecification = selectResourceSpecForContainer();
+ final List<Configuration> configurationsToMerge = new ArrayList<>();
+
evaluatorIdToResourceSpec.put(allocatedContainer.getId(), resourceSpecification);
LOG.info("Container type (" + resourceSpecification.getContainerType()
+ ") allocated, will be used for [" + executorId + "]");
pendingContextIdToResourceSpec.put(executorId, resourceSpecification);
+ configurationsToMerge.add(executorConfiguration);
+
+ // ExecutorMemory handling
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.ExecutorMemoryMb.class, String.valueOf(resourceSpecification.getMemory()))
+ .build()
+ );
+
+ // MaxOffheapRatio handling
+ resourceSpecification.getMaxOffheapRatio().ifPresent(value -> {
+ configurationsToMerge.add(Tang.Factory.getTang().newConfigurationBuilder()
+ .bindNamedParameter(JobConf.MaxOffheapRatio.class, String.valueOf(resourceSpecification.getMaxOffheapRatio()))
Review comment:
changed!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services