[
https://issues.apache.org/jira/browse/YARN-7833?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16583206#comment-16583206
]
Botong Huang edited comment on YARN-7833 at 8/17/18 12:11 AM:
--------------------------------------------------------------
Thanks [~tanujnay] for the patch! I've done the reviewing externally, so
overall looks good to me. I think some of the changes are lost when you prepare
the patch for trunk: "private ResourceManager rm" in SLSRunner.java should be
removed (and all related changes). Let me know if you need help with this.
Some minor things I noticed for v7 patch:
hadoop-sls/pom.xml: indent properly with spaces around <dependency>
Multiple files in the patch: with log4j logger, we don't need
java.text.MessageFormat
ApplicationIdProvider:
Please add some javadocs to this class, as well as DefaultApplicationIdProvider
Since it is a singleton class, please add a private constructor with no param
here.
SLSRunner:
Remove the default value here: private boolean isFederated = false;
line 285: let's do "this.rmProxy = startAMRMProxy(); this.router =
startRouter()", so that the method don't change any global variables.
line 360: move creation of dispatcher outside, and pass into startAMRMProxy()
as a parameter
line 367: duplicate line
line 1100: LOG.error("Error shutting down...", e);
line 1110: line too long, revert this line and line 276. revert the try catch
in line 1194
was (Author: botong):
Thanks [~tanujnay] for the patch! I've done the reviewing externally, so
overall looks good to me. I think some of the changes are lost when you prepare
the patch for trunk: "private ResourceManager rm" should be removed (and all
related changes). Let me know if you need help with this.
Some minor things I noticed for v7 patch:
hadoop-sls/pom.xml: indent properly with spaces around <dependency>
Multiple files in the patch: with log4j logger, we don't need
java.text.MessageFormat
ApplicationIdProvider:
Please add some javadocs to this class, as well as DefaultApplicationIdProvider
Since it is a singleton class, please add a private constructor with no param
here.
SLSRunner:
Remove the default value here: private boolean isFederated = false;
line 285: let's do "this.rmProxy = startAMRMProxy(); this.router =
startRouter()", so that the method don't change any global variables.
line 360: move creation of dispatcher outside, and pass into startAMRMProxy()
as a parameter
line 367: duplicate line
line 1100: LOG.error("Error shutting down...", e);
line 1110: line too long, revert this line and line 276. revert the try catch
in line 1194
> [PERF/TEST] Extend SLS to support simulation of a Federated Environment
> -----------------------------------------------------------------------
>
> Key: YARN-7833
> URL: https://issues.apache.org/jira/browse/YARN-7833
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Carlo Curino
> Assignee: Tanuj Nayak
> Priority: Major
> Attachments: YARN-7833.v1.patch, YARN-7833.v2.patch,
> YARN-7833.v3.patch, YARN-7833.v4.patch, YARN-7833.v5.patch,
> YARN-7833.v6.patch, YARN-7833.v7.patch
>
>
> To develop algorithms for federation, it would be of great help to have a
> version of SLS that supports multi RMs and GPG.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
