[
https://issues.apache.org/jira/browse/MAPREDUCE-5224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13661272#comment-13661272
]
Ivan Mitic commented on MAPREDUCE-5224:
---------------------------------------
Thanks Xi for the patch! I think this is close, have some comments below which
should be easy to address.
1. JobTracker.java: Lines should not exceed 80 chars per Hadoop coding
guidelines.
{code}
FSDataOutputStream out = FileSystem.create(systemDirFs, tmpRestartFile,
filePerm);
{code}
Same comment for other changes in the patch.
2. I really don't think it is necessary to introduce IOException to so many
methods and interfaces for the reasons in this Jira. I would change
JobTracker#getSystemDir() to fallback to the default value and log a warning in
case {{FileSystem.get()}} throws.
3. Should JobTracker#RecoveryManager#checkAndAddJob() use systemDirFs?
4. I would rename the {{JobTracker#fs}} local member to {{defaultFs}} to
signify its meaning and avoid possible confusion in the future. I actually
don’t think you need to keep both defaultFs and systemDirFs as members. The
only other place where you need defaultFs is {{JobHistory#initDone}} and you
should be able to query for it locally.
5. Let's rename the test to TestJobTrackerWithNonDefaultFS
6. What is the expected behavior for TestSysDirOnNonDefaultFS when your code
changes are not applied? Looks like the setUp step is failing. I would prefer
if we could have the test case fail instead.
7. TestSysDirOnNonDefaultFS.java: Please add a more verbose comment on the
intent of the test.
{code}
/**
* Class to test jobtracker's system dir
*/
{code}
8. TestSysDirOnNonDefaultFS.java: Why not let setUp throw the IOException() in
case of an error?
9. TestSysDirOnNonDefaultFS.java: Please use JUnit assertEquals method to
validate that the expected and the retrieved values are equal.
10. TestSysDirOnNonDefaultFS.java: Can we also add validation that mapred
system dir is created in the right place by checking for its existence.
11. Would be good to understand if there are some changes needed to get the
equivalent functionality in YARN. I would be fine with addressing this via a
separate Jira.
> JobTracker should allow the system directory to be in non-default FS
> --------------------------------------------------------------------
>
> Key: MAPREDUCE-5224
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-5224
> Project: Hadoop Map/Reduce
> Issue Type: Bug
> Components: jobtracker
> Reporter: Xi Fang
> Assignee: Xi Fang
> Priority: Minor
> Fix For: 1-win
>
> Attachments: MAPREDUCE-5224.2.patch, MAPREDUCE-5224.patch
>
>
> JobTracker today expects the system directory to be in the default file
> system
> if (fs == null) {
> fs = mrOwner.doAs(new PrivilegedExceptionAction<FileSystem>() {
> public FileSystem run() throws IOException {
> return FileSystem.get(conf);
> }});
> }
> ...
> public String getSystemDir() {
> Path sysDir = new Path(conf.get("mapred.system.dir",
> "/tmp/hadoop/mapred/system"));
> return fs.makeQualified(sysDir).toString();
> }
> In Cloud like Azure the default file system is set as ASV (Windows Azure Blob
> Storage), but we would still like the system directory to be in DFS. We
> should change JobTracker to allow that.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
