hadoop-mapreduce-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ivan Mitic (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (MAPREDUCE-5224) JobTracker should allow the system directory to be in non-default FS
Date Sat, 18 May 2013 04:29:16 GMT

    [ 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

Mime
View raw message