[jira] Commented: (MAPREDUCE-1422) Changing permissions of files/dirs under job-work-dir may be needed sothat cleaning up of job-dir in all mapred-local-directories succeeds always

["Hemanth Yamijala (JIRA)" <ji...@apache.org>]

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

Hemanth Yamijala commented on MAPREDUCE-1422:
---------------------------------------------

I looked at this patch. It seems pretty close. A few very minor comments:

- enable_path_for_cleanup can be marked static as it is not supposed to be used outside taskcontroller.c
- Can we refactor the code in LinuxTaskController.enable{Job|Task}ForCleanup a little bit to avoid duplication to the extent possible:
{code}
private void enablePathForCleanup(String user, PathDeletionContext context, TaskControllerCommands command, List<String> commandArgs) {
  // move common code...
}

void enableJobForCleanup(PathDeletionContext context) {
    // check type of context and cast
    // build args
    List<String> args = buildJobCleanupArgs(context);
    enablePathForCleanup(jContext.user, context, TaskControllerCommands.ENABLE_JOB_FOR_CLEANUP, args);
}
{code}
- I would suggest that instead of combining the test case for job work directory cleanup verification with existing tests, we define a new testcase like testJobDirectory cleanup. In this, we can do the following:
-- Write files into job work directory without write permissions (as done in the patch)
-- Write files into task directory without write permissions - This simulates a scenario where map tasks are not cleaned up until the Job cleanup command comes to the tasktracker.
Then we can use the model of checking for stale paths in the cleanup queue to verify cleanup has happened successfully.
- Also, I don't think we need the new class TestJobLocalDirCleanupWithLinuxTaskController. Note there already exists a class TestLocalizationWithLinuxTaskController that extends TestTaskTrackerLocalization. This would automatically mean that all tests in TestTaskTrackerLocalization get executed along with LinuxTaskController as well. Can you please confirm this ?

> Changing permissions of files/dirs under job-work-dir may be needed sothat cleaning up of job-dir in all mapred-local-directories succeeds always
> -------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-1422
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-1422
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: tasktracker
>            Reporter: Ravi Gummadi
>            Assignee: Amar Kamat
>         Attachments: mapreduce-1422-test-v1.0.patch, mapreduce-1422-v1.4.2.patch
>
>
> After MAPREDUCE-896, if LinuxTaskController is set in config, task-controller binary is launched for changing permissions of taskAttemptDir and taskWorkDir before cleaning up of these directories sothat cleanup will be succeeded even if user had created files/dirs under taskAttemptDir or taskWorkDir with non-writable permissions. Users can't create files/dirs under job-dir directly as we set 2570 for job-dir. But as job-work-dir has 2770 permissions and user can create files/dirs under job-work-dir with non-writable permissions, Changing permissions of files/dirs under job-work-dir may be needed sothat cleaning up of job-dir in all mapred-local-directories succeeds always.

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