[
https://issues.apache.org/jira/browse/YARN-10130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17040101#comment-17040101
]
Szilard Nemeth edited comment on YARN-10130 at 2/19/20 2:36 PM:
----------------------------------------------------------------
Hi [~adam.antal],
Thanks for this patch.
Some comments:
1. Nit: FSConfigToCSConfigArgumentHandler#checkOutputDirDoesNotContainsXmls:
Should be renamed to "checkOutputDirDoesNotContainXml"
2. Would FSConfigToCSConfigArgumentHandler#checkOutputDirDoesNotContainsXmls
work correctly if the String "yarnSiteXmlFile" contains a relative name only? I
mean if -y yarn-site.xml is provided, I'm not sure if the following code block
will work correctly:
{code}
File xmlFile = new File(yarnSiteXmlFile);
File xmlParentFolder = xmlFile.getParentFile();
{code}
In this case, I'm not sure about the value of xmlParentFolder, but maybe I'm
wrong.
3. In FSConfigToCSConfigArgumentHandler#checkOutputDirDoesNotContainsXmls, when
you throw the IllegalArgumentException, you should use yet another static final
string and not the string literal as you used static string for a different
case in checkFileNotInOutputDir. This way, the code could be more consistent.
4. Nit: Comment:
{code:java}
// check whether the output folder does not contain not yarn-site.xml
// neither capacity-scheduler.xml
{code}
Should be: "... nor yarn-site.xml neither..."
5. In checkFileNotInOutputDir: Variable name xmlChild is a bit weird :)
6. In general, I don't get what's going on:
You first call checkOutputDirDoesNotContainsXmls and the first code block
checks if yarn-site.xml can be found in the output directory.
Then you call checkFileNotInOutputDir, once with YARN_SITE_CONFIGURATION_FILE,
then with CS_CONFIGURATION_FILE.
Aren't you checking yarn-site.xml twice, unnecessarily?
Consequently, I don't get what this testcase does:
TestFSConfigToCSConfigArgumentHandler#testYarnSiteOptionInOutputFolder
I think the other 2 testcases are enough.
7. I think the code that throws the exception in checkFileNotInOutputDir has
parameters in a wrong order:
1st param should be: CliOption.OUTPUT_DIR.name
2nd param should be: output
was (Author: snemeth):
Hi [~adam.antal],
Thanks for this patch.
Some comments:
1. Nit: FSConfigToCSConfigArgumentHandler#checkOutputDirDoesNotContainsXmls:
Should be renamed to "checkOutputDirDoesNotContainXml"
2. Would FSConfigToCSConfigArgumentHandler#checkOutputDirDoesNotContainsXmls
work correctly if the String "yarnSiteXmlFile" contains a relative name only? I
mean if -y yarn-site.xml is provided, I'm not sure if the following code block
will work correctly:
{code}
File xmlFile = new File(yarnSiteXmlFile);
File xmlParentFolder = xmlFile.getParentFile();
{code}
In this case, I'm not sure about the value of xmlParentFolder, but maybe I'm
wrong.
3. In FSConfigToCSConfigArgumentHandler#checkOutputDirDoesNotContainsXmls, when
you throw the IllegalArgumentException, you should use yet another static final
string and not the string literal as you used static string for a different
case in checkFileNotInOutputDir. This way, the code could be more consistent.
4. Nit: Comment:
// check whether the output folder does not contain not yarn-site.xml
// neither capacity-scheduler.xml
Should be: "... nor yarn-site.xml neither..."
5. In checkFileNotInOutputDir: Variable name xmlChild is a bit weird :)
6. In general, I don't get what's going on:
You first call checkOutputDirDoesNotContainsXmls and the first code block
checks if yarn-site.xml can be found in the output directory.
Then you call checkFileNotInOutputDir, once with YARN_SITE_CONFIGURATION_FILE,
then with CS_CONFIGURATION_FILE.
Aren't you checking yarn-site.xml twice, unnecessarily?
Consequently, I don't get what this testcase does:
TestFSConfigToCSConfigArgumentHandler#testYarnSiteOptionInOutputFolder
I think the other 2 testcases are enough.
7. I think the code that throws the exception in checkFileNotInOutputDir has
parameters in a wrong order:
1st param should be: CliOption.OUTPUT_DIR.name
2nd param should be: output
> FS-CS converter: Do not allow output dir to be the same as input dir
> --------------------------------------------------------------------
>
> Key: YARN-10130
> URL: https://issues.apache.org/jira/browse/YARN-10130
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Szilard Nemeth
> Assignee: Adam Antal
> Priority: Major
> Attachments: YARN-10130.001.patch, YARN-10130.002.patch,
> YARN-10130.003.patch
>
>
> If the input dir where fair-scheduler.xml / yarn-site.xml sits is the same as
> the output dir (defined by the -o switch), the fs2cs tool overwrites the
> source config files, i.e. yarn-site.xml.
> Reproduce this is easy, just run fs2cs tool with this command:
> {code:java}
> /bin/yarn fs2cs --cluster-resource memory-mb=18044928,vcores=16
> --no-terminal-rule-check -y yarn-site.xml -f fair-scheduler.xml -o .
> {code}
> The following (or similar) is emitted by the tool:
> {code:java}
> WARNING: YARN_OPTS has been replaced by HADOOP_OPTS. Using value of
> YARN_OPTS.WARNING: YARN_OPTS has been replaced by HADOOP_OPTS. Using value of
> YARN_OPTS.20/02/10 12:51:42 INFO converter.FSConfigToCSConfigConverter:
> Output directory for yarn-site.xml and capacity-scheduler.xml is: .20/02/10
> 12:51:42 INFO converter.FSConfigToCSConfigConverter: Conversion rules file is
> not defined, using default conversion config!20/02/10 12:51:42 ERROR
> conf.Configuration: error parsing conf
> yarn-site.xmlcom.ctc.wstx.exc.WstxEOFException: Unexpected EOF in prolog at
> [row,col,system-id]: [1,0,"yarn-site.xml"] at
> com.ctc.wstx.sr.StreamScanner.throwUnexpectedEOF(StreamScanner.java:687) at
> com.ctc.wstx.sr.BasicStreamReader.handleEOF(BasicStreamReader.java:2220) at
> com.ctc.wstx.sr.BasicStreamReader.nextFromProlog(BasicStreamReader.java:2126)
> at com.ctc.wstx.sr.BasicStreamReader.next(BasicStreamReader.java:1181) at
> org.apache.hadoop.conf.Configuration$Parser.parseNext(Configuration.java:3343)
> at
> org.apache.hadoop.conf.Configuration$Parser.parse(Configuration.java:3137) at
> org.apache.hadoop.conf.Configuration.loadResource(Configuration.java:3030) at
> org.apache.hadoop.conf.Configuration.loadResources(Configuration.java:2996)
> at org.apache.hadoop.conf.Configuration.getProps(Configuration.java:2871) at
> org.apache.hadoop.conf.Configuration.set(Configuration.java:1389) at
> org.apache.hadoop.conf.Configuration.set(Configuration.java:1361) at
> org.apache.hadoop.conf.Configuration.setBoolean(Configuration.java:1702) at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.converter.FSConfigToCSConfigConverter.createConfiguration(FSConfigToCSConfigConverter.java:166)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.converter.FSConfigToCSConfigConverter.convert(FSConfigToCSConfigConverter.java:98)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.converter.FSConfigToCSConfigArgumentHandler.parseAndConvert(FSConfigToCSConfigArgumentHandler.java:137)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.converter.FSConfigToCSConfigConverterMain.main(FSConfigToCSConfigConverterMain.java:40)20/02/10
> 12:51:42 ERROR converter.FSConfigToCSConfigConverterMain: Error while
> starting FS configuration conversion!
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
