[ 
https://issues.apache.org/jira/browse/MAPREDUCE-1778?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12876549#action_12876549
 ] 

Amar Kamat commented on MAPREDUCE-1778:
---------------------------------------

Some comments 
1) Bug related:
  1.1) I think the newly created completed-jobstatus-store directory should 
assume permissions from the parent dir instead of hardcoding it to 750.
  1.2) If completed-jobstatus-store already exists then the only thing that 
should be tested/verified is whether its writable+readable by jobtracker or 
not. It doesnt really matter what the parent/group/other's permissions really 
are as long as the JobTracker can get its work (read+write) done.

2) Coding related
  2.1) Make sure that the code is aligned within 80 characters limit (i.e 
column width)

3) Testcase related
  3.1) Use log4j instead of mortbay logger
  3.2) Add descriptive javadoc for the testcase. Mention that the scope of the 
test is only limited to JobTracker's usage of CompletedJobStatusStore.
  3.3) I still see some commented out code in the testcase. Plz remove it.
  3.4) I would prefer using the existing testcase (i.e 
TestJobStatusPersistency.java) instead of introducing a new testcase (i.e 
TestJobStatusStoreConfig.java). See http://tinyurl.com/24kzetd.
  3.5) [Minor] Indentation related : See line 166,167 of the patch

> CompletedJobStatusStore initialization should fail if 
> {mapred.job.tracker.persist.jobstatus.dir} is unwritable
> --------------------------------------------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-1778
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-1778
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: jobtracker
>            Reporter: Amar Kamat
>            Assignee: Amar Kamat
>         Attachments: mapred-1778-1.patch, mapred-1778-2.patch, 
> mapred-1778.patch
>
>
> If {mapred.job.tracker.persist.jobstatus.dir} points to an unwritable 
> location or mkdir of {mapred.job.tracker.persist.jobstatus.dir} fails, then 
> CompletedJobStatusStore silently ignores the failure and disables 
> CompletedJobStatusStore. Ideally the JobTracker should bail out early 
> indicating a misconfiguration.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to