[jira] [Commented] (MAPREDUCE-4680) Job history cleaner should only check timestamps of files in old enough directories

["Sandy Ryza (JIRA)" <ji...@apache.org>]

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

Sandy Ryza commented on MAPREDUCE-4680:
---------------------------------------

Thanks Robert.  The code looks good.  Just a couple more things:
* Can you add javadoc to the top of the method explaining what it's doing
* Can you add short comments above the if statements like "(year < cYear || month <= cMonth)" explaining why they work.
* Can you put the "catch"s on the same lines as the try block close braces
* Can you make it clear that we're ignoring exceptions because they mean we found something in the directory that doesn't fit our format? 

> Job history cleaner should only check timestamps of files in old enough directories
> -----------------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-4680
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4680
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: jobhistoryserver
>    Affects Versions: 2.0.0-alpha
>            Reporter: Sandy Ryza
>            Assignee: Robert Kanter
>         Attachments: MAPREDUCE-4680.patch, MAPREDUCE-4680.patch, MAPREDUCE-4680.patch
>
>
> Job history files are stored in yyyy/mm/dd folders.  Currently, the job history cleaner checks the modification date of each file in every one of these folders to see whether it's past the maximum age.  The load on HDFS could be reduced by only checking the ages of files in directories that are old enough, as determined by their name.



--
This message was sent by Atlassian JIRA
(v6.1#6144)