Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: tasktracker
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Hide
      Fixed TaskTracker and related classes so as to set correct and most restrictive access control for DistributedCache files/archives.
       - To do this, it changed the directory structure of per-job local files on a TaskTracker to the following:
      $mapred.local.dir
         `-- taskTracker
              `-- $user
                     |- distcache
                     `-- jobcache
       - Distributed cache files/archives are now user-owned by the job-owner and the group-owned by the special group-owner of the task-controller binary. The files/archives are set most private permissions possible, and as soon as possible, immediately after the files/dirs are first localized on the TT.
       - As depicted by the new directory structure, a directory corresponding to each user is created on each TT when that particular user's first task are assigned to the corresponding TT. These user directories remain on the TT forever are not cleaned when unused, which is targeted to be fixed via MAPREDUCE-1019.
       - The distributed cache files are now accessible _only_ by the user who first localized them. Sharing of these files across users is no longer possible, but is targeted for future versions via MAPREDUCE-744.
      Show
      Fixed TaskTracker and related classes so as to set correct and most restrictive access control for DistributedCache files/archives.  - To do this, it changed the directory structure of per-job local files on a TaskTracker to the following: $mapred.local.dir    `-- taskTracker         `-- $user                |- distcache                `-- jobcache  - Distributed cache files/archives are now user-owned by the job-owner and the group-owned by the special group-owner of the task-controller binary. The files/archives are set most private permissions possible, and as soon as possible, immediately after the files/dirs are first localized on the TT.  - As depicted by the new directory structure, a directory corresponding to each user is created on each TT when that particular user's first task are assigned to the corresponding TT. These user directories remain on the TT forever are not cleaned when unused, which is targeted to be fixed via MAPREDUCE-1019 .  - The distributed cache files are now accessible _only_ by the user who first localized them. Sharing of these files across users is no longer possible, but is targeted for future versions via MAPREDUCE-744 .
    1. MAPREDUCE-856-20090820.txt
      116 kB
      Vinod Kumar Vavilapalli
    2. MAPREDUCE-856-20090821.txt
      116 kB
      Vinod Kumar Vavilapalli
    3. MAPREDUCE-856-20090825.3.txt
      138 kB
      Vinod Kumar Vavilapalli
    4. MAPREDUCE-856-20090827.txt
      148 kB
      Vinod Kumar Vavilapalli
    5. MAPREDUCE-856-20090903.txt
      149 kB
      Vinod Kumar Vavilapalli
    6. MAPREDUCE-856-20090904.1.txt
      151 kB
      Vinod Kumar Vavilapalli
    7. MAPREDUCE-856-20090904.txt
      150 kB
      Vinod Kumar Vavilapalli
    8. MAPREDUCE-856-20090907.1.txt
      153 kB
      Vinod Kumar Vavilapalli
    9. MAPREDUCE-856-20090907.txt
      153 kB
      Vinod Kumar Vavilapalli
    10. MAPREDUCE-856-20090908.txt
      153 kB
      Vinod Kumar Vavilapalli
    11. MAPREDUCE-856-20090908-y20.txt
      151 kB
      Hemanth Yamijala

      Issue Links

        Activity

        Arun C Murthy created issue -
        Arun C Murthy made changes -
        Field Original Value New Value
        Component/s mapred [ 12310690 ]
        Component/s security [ 12312526 ]
        Hemanth Yamijala made changes -
        Assignee Arun C Murthy [ acmurthy ]
        Vinod Kumar Vavilapalli made changes -
        Link This issue is related to HADOOP-4490 [ HADOOP-4490 ]
        Hide
        Vinod Kumar Vavilapalli added a comment -

        I've started looking at this.

        Some of the discussions about distributed cache already happened on HADOOP-4490. HADOOP-4491 takes care of setting most restrictive permissions possible for all the files/directories related to a job. This jira is aimed at secure permissions for files under the distributed cache.

        State of the art

        • One important point with the distributed cache is that it currently can be shared across jobs.
        • The files in the cache are localized once per node and used subsequently by tasks using symlinks.
        • The cache is purged/cleaned up
          • when the cache grows beyond a configurable size limit,
          • on TaskTracker start or reinitialize.

        So, unless one of the above situations is triggered, multiple jobs of the same user or even mulitple users can by default use files that are already localized by a previous job. The sharing depends on how TT generally creates its files(its umask).

        (Side note: In the current code, the quota is checked after localization and not before. Also if the reference count of none of the files is zero, neither do we purge any files, nor do we stop localizing more files. Isn't this a bug?)

        Show
        Vinod Kumar Vavilapalli added a comment - I've started looking at this. Some of the discussions about distributed cache already happened on HADOOP-4490 . HADOOP-4491 takes care of setting most restrictive permissions possible for all the files/directories related to a job. This jira is aimed at secure permissions for files under the distributed cache. State of the art One important point with the distributed cache is that it currently can be shared across jobs. The files in the cache are localized once per node and used subsequently by tasks using symlinks. The cache is purged/cleaned up when the cache grows beyond a configurable size limit, on TaskTracker start or reinitialize. So, unless one of the above situations is triggered, multiple jobs of the same user or even mulitple users can by default use files that are already localized by a previous job. The sharing depends on how TT generally creates its files(its umask). (Side note: In the current code, the quota is checked after localization and not before. Also if the reference count of none of the files is zero, neither do we purge any files, nor do we stop localizing more files. Isn't this a bug?)
        Hide
        Vinod Kumar Vavilapalli added a comment -

        This issue's focus

        Given the state of the art, what do we mean by restrictive permissions for files in distributed cache?

        Case I: The cache files are completely private to the job-owner

        • The job owner wants his/her files only for himself/herself and doesn't want access to anyone else.
        • This means 700 permissions and owned by the job owner, so only the job owner can access it.
        • To facilitate the above, the cache should be localized under a directory with user name.
        • The files should be usable by subsequent jobs of the same user.
        • The configurable size limit of the cache or in other words the disk quota for the cache files should be per user.
        • Because the files are owned by the user, we will need a task-controller process launch during cleaning-up/purging.

        Case II: The job owner is fine with sharing his/her distributed cache files with other users

        • A possible use case for this, as mentioned on HADOOP-4490, is a multiple of users, perhaps working on the same project that requires the same data files, want to share these files across multiple jobs of possibly other users.
        • The above would mean _55 permissions for everyone.
        • The files should be localized under a common directory not associated to any user name.
        • There should be a configuration knob(per cache URI?) to be specified by the user so as to distinguish these files from the ones that belong to CASE I.
        • The disk quota for the cache files should be a global one and not specific to the user.
        • The files can be owned by the job owner or the TT itself.
          • Having TT own the files makes it easy for cleaning up.
          • If the ownership is with the job-owner, we would need a task-controller process launch. We we also need user information with the files, perhaps through a sub-directory associated with a user name.

        So, do we want only CASE I or only CASE II or a combination of both? Thoughts/comments?

        Show
        Vinod Kumar Vavilapalli added a comment - This issue's focus Given the state of the art, what do we mean by restrictive permissions for files in distributed cache? Case I: The cache files are completely private to the job-owner The job owner wants his/her files only for himself/herself and doesn't want access to anyone else. This means 700 permissions and owned by the job owner, so only the job owner can access it. To facilitate the above, the cache should be localized under a directory with user name. The files should be usable by subsequent jobs of the same user. The configurable size limit of the cache or in other words the disk quota for the cache files should be per user. Because the files are owned by the user, we will need a task-controller process launch during cleaning-up/purging. Case II: The job owner is fine with sharing his/her distributed cache files with other users A possible use case for this, as mentioned on HADOOP-4490 , is a multiple of users, perhaps working on the same project that requires the same data files, want to share these files across multiple jobs of possibly other users. The above would mean _55 permissions for everyone. The files should be localized under a common directory not associated to any user name. There should be a configuration knob(per cache URI?) to be specified by the user so as to distinguish these files from the ones that belong to CASE I. The disk quota for the cache files should be a global one and not specific to the user. The files can be owned by the job owner or the TT itself. Having TT own the files makes it easy for cleaning up. If the ownership is with the job-owner, we would need a task-controller process launch. We we also need user information with the files, perhaps through a sub-directory associated with a user name. So, do we want only CASE I or only CASE II or a combination of both? Thoughts/comments?
        Hide
        Vinod Kumar Vavilapalli added a comment -

        One more (hybrid) case is possible.

        CASE III: When job owner wants to share with a specific list of users that he mentions.

        • A minor variant of CASE II.
        • Job owner either specifies the list of users or a group of users with which he wants to share libraries.
        • With most of the other things being the same as in CASE II, the api in DistributedCache should make sure that the access is restricted only to the list of users or the group specified.
        • Even here, the access list should be per cache uri.
        Show
        Vinod Kumar Vavilapalli added a comment - One more (hybrid) case is possible. CASE III: When job owner wants to share with a specific list of users that he mentions. A minor variant of CASE II. Job owner either specifies the list of users or a group of users with which he wants to share libraries. With most of the other things being the same as in CASE II, the api in DistributedCache should make sure that the access is restricted only to the list of users or the group specified. Even here, the access list should be per cache uri.
        Hide
        Owen O'Malley added a comment -

        I think the default should be case 1, with completely private distributed caches.

        I believe that we don't need sharing with a group, and can probably delay doing case 2 (global sharing) until later.

        Thoughts?

        Show
        Owen O'Malley added a comment - I think the default should be case 1, with completely private distributed caches. I believe that we don't need sharing with a group, and can probably delay doing case 2 (global sharing) until later. Thoughts?
        Hide
        Vinod Kumar Vavilapalli added a comment -

        One comment that came out of a Hemanth's discussion with Devaraj/Owen is w.r.t access of user files on the file system by the DistributedCache api on the TT. The user files will be assumed to be securely owned by the user and so TT has the onus to use the job's configuration and hence the user's identity to download files from the file system.

        Show
        Vinod Kumar Vavilapalli added a comment - One comment that came out of a Hemanth's discussion with Devaraj/Owen is w.r.t access of user files on the file system by the DistributedCache api on the TT. The user files will be assumed to be securely owned by the user and so TT has the onus to use the job's configuration and hence the user's identity to download files from the file system.
        Owen O'Malley made changes -
        Component/s mapred [ 12310690 ]
        Hide
        Vinod Kumar Vavilapalli added a comment -

        Talked with Milind offline, and he too agrees to the proposal of doing Case I first and Case II later. He adds that Case II nevertheless is useful and is on the long term wish-list.

        Show
        Vinod Kumar Vavilapalli added a comment - Talked with Milind offline, and he too agrees to the proposal of doing Case I first and Case II later. He adds that Case II nevertheless is useful and is on the long term wish-list.
        Vinod Kumar Vavilapalli made changes -
        Link This issue blocks MAPREDUCE-744 [ MAPREDUCE-744 ]
        Hide
        Vinod Kumar Vavilapalli added a comment -

        Created MAPREDUCE-744 for Case II.

        Show
        Vinod Kumar Vavilapalli added a comment - Created MAPREDUCE-744 for Case II.
        Vinod Kumar Vavilapalli made changes -
        Link This issue is blocked by MAPREDUCE-711 [ MAPREDUCE-711 ]
        Vinod Kumar Vavilapalli made changes -
        Link This issue is blocked by MAPREDUCE-476 [ MAPREDUCE-476 ]
        Hide
        Hemanth Yamijala added a comment -

        Had a chat with Owen to get more details. Here is the summary:

        • To download the files from distributed cache, we will use the user's JobConf for getting the file system handle. Owen feels any changes to Hadoop security will be abstracted here and hence we will not be affected.
        • In this JIRA, we'll manage a per user cache where the downloaded cache files will be managed securely as done in MAPREDUCE-842 (ownership set to user, group ownership to the tasktracker user, 570 permissions).
        • The per user cache will be under $ttroot/users/$user/archive
        • Owen suggested we can change the jobcache also to be under $ttroot/users/$user/jobcache as part of this jira. (However, we will not change the direction of MAPREDUCE-842 which is quite close to the finish line now)
        • There'll be symlinks from the task directories to the distributed cache files.
        • The current paging policy which sets a soft limit on the size of the distributed cache on disk will remain unchanged. We discussed there could be potentially work in the future to make this a hard limit, which could even have impacts on scheduling in the sense that the scheduler should schedule tasks to be within the limits). But this is certainly the scope of another jira.
        Show
        Hemanth Yamijala added a comment - Had a chat with Owen to get more details. Here is the summary: To download the files from distributed cache, we will use the user's JobConf for getting the file system handle. Owen feels any changes to Hadoop security will be abstracted here and hence we will not be affected. In this JIRA, we'll manage a per user cache where the downloaded cache files will be managed securely as done in MAPREDUCE-842 (ownership set to user, group ownership to the tasktracker user, 570 permissions). The per user cache will be under $ttroot/users/$user/archive Owen suggested we can change the jobcache also to be under $ttroot/users/$user/jobcache as part of this jira. (However, we will not change the direction of MAPREDUCE-842 which is quite close to the finish line now) There'll be symlinks from the task directories to the distributed cache files. The current paging policy which sets a soft limit on the size of the distributed cache on disk will remain unchanged. We discussed there could be potentially work in the future to make this a hard limit, which could even have impacts on scheduling in the sense that the scheduler should schedule tasks to be within the limits). But this is certainly the scope of another jira.
        Vinod Kumar Vavilapalli made changes -
        Assignee Vinod K V [ vinodkv ]
        Vinod Kumar Vavilapalli made changes -
        Parent HADOOP-4487 [ 12407018 ]
        Issue Type Sub-task [ 7 ] Bug [ 1 ]
        Vinod Kumar Vavilapalli made changes -
        Project Hadoop Common [ 12310240 ] Hadoop Map/Reduce [ 12310941 ]
        Key HADOOP-4493 MAPREDUCE-856
        Component/s tasktracker [ 12312906 ]
        Component/s security [ 12312526 ]
        Vinod Kumar Vavilapalli made changes -
        Parent MAPREDUCE-563 [ 12428534 ]
        Issue Type Bug [ 1 ] Sub-task [ 7 ]
        Hide
        Vinod Kumar Vavilapalli added a comment -

        Attaching patch. It has to be applied over the latest one at MAPREDUCE-476. This does the following:

        • Changes dir structure to the following:
          $mapred.local.dir
             `-- taskTracker
                  `-- $user
                         |- distcache
                         |
                         `-- jobcache
          
        • Adds a new initializeDistributedCache(InitializationContext context) to secure distributed cache files via TaskRunner
        • Adds a new INITIALIZE_DISTRIBUTEDCACHE command for LinuxTaskController binary which takes a user name as argument and sets private permissions for the corresponding distcache files in $mapred.local.dir/taskTracker/$user/distcache
        • Adds a new INITIALIZE_USER command to set secure permissions for the $mapred.local.dir/taskTracker/$user directories.
        • Adds user as part of Task serialization as TT needs user name even before job-conf is localized. The user-name is needed for creating user specific directories $mapred.local.dir/taskTracker/$user.
        • Moves some of the old and newly added localization code in TaskTracker.java to org.apache.hadoop.mapreduce.server.tasktracker.Localizer as per MAPREDUCE-759 and MAPREDUCE-303. Had to change the scope of few things to public, but marked them only for internal use.
        • Adds functionality to clean up stale user directories whenever possible.
        • Adds a new test in TestTaskTrackerLocalization to verify cleanup of task files when needed.
        • Adds a new testcase TestTrackerDistributedCacheManagerWithLinuxTaskController to verify permissions set on distributed cache files by LinuxTaskController
        Show
        Vinod Kumar Vavilapalli added a comment - Attaching patch. It has to be applied over the latest one at MAPREDUCE-476 . This does the following: Changes dir structure to the following: $mapred.local.dir `-- taskTracker `-- $user |- distcache | `-- jobcache Adds a new initializeDistributedCache(InitializationContext context) to secure distributed cache files via TaskRunner Adds a new INITIALIZE_DISTRIBUTEDCACHE command for LinuxTaskController binary which takes a user name as argument and sets private permissions for the corresponding distcache files in $mapred.local.dir/taskTracker/$user/distcache Adds a new INITIALIZE_USER command to set secure permissions for the $mapred.local.dir/taskTracker/$user directories. Adds user as part of Task serialization as TT needs user name even before job-conf is localized. The user-name is needed for creating user specific directories $mapred.local.dir/taskTracker/$user. Moves some of the old and newly added localization code in TaskTracker.java to org.apache.hadoop.mapreduce.server.tasktracker.Localizer as per MAPREDUCE-759 and MAPREDUCE-303 . Had to change the scope of few things to public, but marked them only for internal use. Adds functionality to clean up stale user directories whenever possible. Adds a new test in TestTaskTrackerLocalization to verify cleanup of task files when needed. Adds a new testcase TestTrackerDistributedCacheManagerWithLinuxTaskController to verify permissions set on distributed cache files by LinuxTaskController
        Vinod Kumar Vavilapalli made changes -
        Attachment MAPREDUCE-856-20090820.txt [ 12417112 ]
        Hide
        Vinod Kumar Vavilapalli added a comment -

        The previous patch had git prefixes. Attaching a new patch leaving those behind.

        Show
        Vinod Kumar Vavilapalli added a comment - The previous patch had git prefixes. Attaching a new patch leaving those behind.
        Vinod Kumar Vavilapalli made changes -
        Attachment MAPREDUCE-856-20090821.txt [ 12417243 ]
        Hide
        Hemanth Yamijala added a comment -

        Looked at the patch. I have a few comments:

        • Make Localizer an instance class, as in general, that's a more flexible design, and also there's state that the localizer is needing to maintain anyway.
        • I would recommend initializeUserDirs to pass the taskcontroller instead of tasktracker, as the entire tasktracker interface is not needed by the localizer atleast now.
        • In HADOOP-4491, if the user directory cannot be created on any disk, we were failing localization. I think that's a useful feature to have.
          -Synchronization w.r.to user localization needs to be looked at.
          • It is possible right now that when user localization is in progress for a user, another task for the same user could get launched before the localization completes.
          • Also, the object on which we are locking - is it guaranteed that it is a unique instance for every user ?
        • Race condition exists between creation and deletion of user directories. Say a job requires a user dir and has not yet localized files (and consequently hasn't acquired the synchronization lock. At that time if deletion starts, it could delete the user dir.
        • Also, I think it will be good to check for cleaning up user directories on a much slower pace as they involve some costly operations.
        • I think JobConf.setUserAndGroupNamesForJob need not be static. Also, it would be nice to document that this is mainly used in test cases.
        • User directory can be 570. So also distributed cache directory (no need even for setuid, right ?)
        • The changes in MAPREDUCE-871 need to be synced up in this patch as well.
        • Some tests like TestTaskControllerSetup are disabled. Can you please enable them back.
        • Permission checks for user directory and jobcache and archive directory permissions needed.
        • Test cases should also confirm directory paths in localized distributed cache paths are being set to the right permissions.
        • Can we use testManagerFlow to have templates that can be overridden by the LinuxTaskController test class.
        Show
        Hemanth Yamijala added a comment - Looked at the patch. I have a few comments: Make Localizer an instance class, as in general, that's a more flexible design, and also there's state that the localizer is needing to maintain anyway. I would recommend initializeUserDirs to pass the taskcontroller instead of tasktracker, as the entire tasktracker interface is not needed by the localizer atleast now. In HADOOP-4491 , if the user directory cannot be created on any disk, we were failing localization. I think that's a useful feature to have. -Synchronization w.r.to user localization needs to be looked at. It is possible right now that when user localization is in progress for a user, another task for the same user could get launched before the localization completes. Also, the object on which we are locking - is it guaranteed that it is a unique instance for every user ? Race condition exists between creation and deletion of user directories. Say a job requires a user dir and has not yet localized files (and consequently hasn't acquired the synchronization lock. At that time if deletion starts, it could delete the user dir. Also, I think it will be good to check for cleaning up user directories on a much slower pace as they involve some costly operations. I think JobConf.setUserAndGroupNamesForJob need not be static. Also, it would be nice to document that this is mainly used in test cases. User directory can be 570. So also distributed cache directory (no need even for setuid, right ?) The changes in MAPREDUCE-871 need to be synced up in this patch as well. Some tests like TestTaskControllerSetup are disabled. Can you please enable them back. Permission checks for user directory and jobcache and archive directory permissions needed. Test cases should also confirm directory paths in localized distributed cache paths are being set to the right permissions. Can we use testManagerFlow to have templates that can be overridden by the LinuxTaskController test class.
        Hide
        Hemanth Yamijala added a comment -

        User directory can be 570. So also distributed cache directory (no need even for setuid, right ?)

        I meant setgid.. However, that may be required, as we realized in an internal discussion.

        Show
        Hemanth Yamijala added a comment - User directory can be 570. So also distributed cache directory (no need even for setuid, right ?) I meant setgid.. However, that may be required, as we realized in an internal discussion.
        Hide
        Vinod Kumar Vavilapalli added a comment -

        Updated patch, synching with latest patches at MAPREDUCE-476 and MAPREDUCE-871 which are blockers for this issue, the later also because of clash of changes.

        Included all of the above review comments except one.

        The one related to cleanup of stale user directories is not going to be done as part of this issue. As implementation went on, it became more and more complicated, the latest complexity added due to the fact that cleanup of distributed cache files only removes the files and leaves behind a lot of empty path components behind. It is still doable but stretches the scope of the current issue a bit. Stale dirs cleanup can be done as part of a follow-up issue.

        Show
        Vinod Kumar Vavilapalli added a comment - Updated patch, synching with latest patches at MAPREDUCE-476 and MAPREDUCE-871 which are blockers for this issue, the later also because of clash of changes. Included all of the above review comments except one. The one related to cleanup of stale user directories is not going to be done as part of this issue. As implementation went on, it became more and more complicated, the latest complexity added due to the fact that cleanup of distributed cache files only removes the files and leaves behind a lot of empty path components behind. It is still doable but stretches the scope of the current issue a bit. Stale dirs cleanup can be done as part of a follow-up issue.
        Vinod Kumar Vavilapalli made changes -
        Attachment MAPREDUCE-856-20090825.3.txt [ 12417592 ]
        Vinod Kumar Vavilapalli made changes -
        Link This issue is blocked by MAPREDUCE-871 [ MAPREDUCE-871 ]
        Hide
        Vinod Kumar Vavilapalli added a comment -

        Running tests locally as the blocker are not yet checked in.

        Show
        Vinod Kumar Vavilapalli added a comment - Running tests locally as the blocker are not yet checked in.
        Hide
        Vinod Kumar Vavilapalli added a comment -

        Updated patch fixing few test failures that were observed in an offline test-run.

        Show
        Vinod Kumar Vavilapalli added a comment - Updated patch fixing few test failures that were observed in an offline test-run.
        Vinod Kumar Vavilapalli made changes -
        Attachment MAPREDUCE-856-20090827.txt [ 12417863 ]
        Hide
        Hemanth Yamijala added a comment -

        Looked at the new patch. Some comments:

        • If the user directory cannot be created on any disk, we should fail localization. (This is not incorporated)
        • Synchronization is still necessary when creating user directories. Consider a case where two tasks from the same user from two different jobs are being localized at the same time. This could cause failures in making the directories etc. Also note that bringing back this synchronization needs to incorporate comments related to synchronization that were raised before.
        • The changes in JobClient might cause a regression. Not sure about this. In the old code, the call to getUGI was done in configureCommandLineOptions much early on, with the result that the jobConf parameter was changed to set the hadoop.job.ugi. This conf object is used for other purposes like uploading jar files etc. Would this be a problem in the new code, because this call is now being set much later.
        • TestMiniMRWithDFS.checkTaskDirectories seems to be more generic than needed and hence more complicated. For e.g. if we currently don't seem to need multi-user support - so can we drop it. The advantage is that it makes the code simpler. Right now, it seems too generic because it needs to support these generic cases.
        Show
        Hemanth Yamijala added a comment - Looked at the new patch. Some comments: If the user directory cannot be created on any disk, we should fail localization. (This is not incorporated) Synchronization is still necessary when creating user directories. Consider a case where two tasks from the same user from two different jobs are being localized at the same time. This could cause failures in making the directories etc. Also note that bringing back this synchronization needs to incorporate comments related to synchronization that were raised before. The changes in JobClient might cause a regression. Not sure about this. In the old code, the call to getUGI was done in configureCommandLineOptions much early on, with the result that the jobConf parameter was changed to set the hadoop.job.ugi. This conf object is used for other purposes like uploading jar files etc. Would this be a problem in the new code, because this call is now being set much later. TestMiniMRWithDFS.checkTaskDirectories seems to be more generic than needed and hence more complicated. For e.g. if we currently don't seem to need multi-user support - so can we drop it. The advantage is that it makes the code simpler. Right now, it seems too generic because it needs to support these generic cases.
        Hide
        Vinod Kumar Vavilapalli added a comment -

        Updated patch including the above comments.

        Show
        Vinod Kumar Vavilapalli added a comment - Updated patch including the above comments.
        Vinod Kumar Vavilapalli made changes -
        Attachment MAPREDUCE-856-20090903.txt [ 12418592 ]
        Vinod Kumar Vavilapalli made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Fix Version/s 0.21.0 [ 12314045 ]
        Vinod Kumar Vavilapalli made changes -
        Link This issue blocks MAPREDUCE-890 [ MAPREDUCE-890 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12418592/MAPREDUCE-856-20090903.txt
        against trunk revision 811134.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 23 new or modified tests.

        -1 patch. The patch command could not apply the patch.

        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/39/console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12418592/MAPREDUCE-856-20090903.txt against trunk revision 811134. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 23 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/39/console This message is automatically generated.
        Vinod Kumar Vavilapalli made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Vinod Kumar Vavilapalli added a comment -

        Updating patch synch'ed with trunk.

        Show
        Vinod Kumar Vavilapalli added a comment - Updating patch synch'ed with trunk.
        Vinod Kumar Vavilapalli made changes -
        Attachment MAPREDUCE-856-20090904.txt [ 12418615 ]
        Vinod Kumar Vavilapalli made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12418615/MAPREDUCE-856-20090904.txt
        against trunk revision 811134.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 23 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        -1 javac. The patch appears to cause tar ant target to fail.

        -1 findbugs. The patch appears to cause Findbugs to fail.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed core unit tests.

        -1 contrib tests. The patch failed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/7/testReport/
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/7/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/7/console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12418615/MAPREDUCE-856-20090904.txt against trunk revision 811134. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 23 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The patch appears to cause tar ant target to fail. -1 findbugs. The patch appears to cause Findbugs to fail. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/7/testReport/ Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/7/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/7/console This message is automatically generated.
        Vinod Kumar Vavilapalli made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Vinod Kumar Vavilapalli added a comment -

        Updated patch fixing the compilation. Had to modify MapOuput class's constructor to use the (deprecated) JobConf instead of Configuration as the code needs to get the user name of the job. Checked with Jothi, who wrote this class, to ensure it's fine doing this.

        Show
        Vinod Kumar Vavilapalli added a comment - Updated patch fixing the compilation. Had to modify MapOuput class's constructor to use the (deprecated) JobConf instead of Configuration as the code needs to get the user name of the job. Checked with Jothi, who wrote this class, to ensure it's fine doing this.
        Vinod Kumar Vavilapalli made changes -
        Attachment MAPREDUCE-856-20090904.1.txt [ 12418620 ]
        Vinod Kumar Vavilapalli made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Vinod Kumar Vavilapalli added a comment -

        In general, it seems like JobConf is one of the only few deprecated classes that is being used by the classes in the new 'mapreduce' package. Apparently, as Amareshwari says, JobContext is the object that should be used by the mapred framework instead of JobConf, but it cannot be done right away. At any rate, we should really think of ways as to how we are going to move forward regarding this. Will open an issue for the same.

        Show
        Vinod Kumar Vavilapalli added a comment - In general, it seems like JobConf is one of the only few deprecated classes that is being used by the classes in the new 'mapreduce' package. Apparently, as Amareshwari says, JobContext is the object that should be used by the mapred framework instead of JobConf, but it cannot be done right away. At any rate, we should really think of ways as to how we are going to move forward regarding this. Will open an issue for the same.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12418620/MAPREDUCE-856-20090904.1.txt
        against trunk revision 811134.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 23 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/8/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/8/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/8/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/8/console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12418620/MAPREDUCE-856-20090904.1.txt against trunk revision 811134. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 23 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/8/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/8/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/8/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/8/console This message is automatically generated.
        Hide
        Hemanth Yamijala added a comment -

        I verified the changes. Only comment is that in the changes related to synchronization of user localization, we are repeating work related to a user everytime job localization happens. A suggestion is to keep the synchronization on user name, but have the value to be a state variable that can indicate the status of localization and check that before beginning to localize.

        Show
        Hemanth Yamijala added a comment - I verified the changes. Only comment is that in the changes related to synchronization of user localization, we are repeating work related to a user everytime job localization happens. A suggestion is to keep the synchronization on user name, but have the value to be a state variable that can indicate the status of localization and check that before beginning to localize.
        Vinod Kumar Vavilapalli made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Vinod Kumar Vavilapalli added a comment -

        Attaching patch that fixes the problem mentioned w.r.t repetitive localization of users. Modified tests to verify the same.

        Show
        Vinod Kumar Vavilapalli added a comment - Attaching patch that fixes the problem mentioned w.r.t repetitive localization of users. Modified tests to verify the same.
        Vinod Kumar Vavilapalli made changes -
        Attachment MAPREDUCE-856-20090907.txt [ 12418789 ]
        Hide
        Vinod Kumar Vavilapalli added a comment -

        Updated patch fixing the test failures reported by Hudson.

        Show
        Vinod Kumar Vavilapalli added a comment - Updated patch fixing the test failures reported by Hudson.
        Vinod Kumar Vavilapalli made changes -
        Attachment MAPREDUCE-856-20090907.1.txt [ 12418791 ]
        Vinod Kumar Vavilapalli made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12418791/MAPREDUCE-856-20090907.1.txt
        against trunk revision 812002.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 23 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/10/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/10/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/10/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/10/console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12418791/MAPREDUCE-856-20090907.1.txt against trunk revision 812002. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 23 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/10/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/10/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/10/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/10/console This message is automatically generated.
        Vinod Kumar Vavilapalli made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Vinod Kumar Vavilapalli added a comment -

        Attaching patch that fixes the test-cases failures. I couldn't pin point the exact reason for the place of error but it may be that multiple TTs run in threads in the same JVM in tests, and the data structure 'localizedUsers' is static; so initialization of user is happening in only the first TT and not in the others. The tests failed without the changes in this patch on my local machine too and passed with the patch.

        Also, cleaned up tests to prevent other potential problems.

        Show
        Vinod Kumar Vavilapalli added a comment - Attaching patch that fixes the test-cases failures. I couldn't pin point the exact reason for the place of error but it may be that multiple TTs run in threads in the same JVM in tests, and the data structure 'localizedUsers' is static; so initialization of user is happening in only the first TT and not in the others. The tests failed without the changes in this patch on my local machine too and passed with the patch. Also, cleaned up tests to prevent other potential problems.
        Vinod Kumar Vavilapalli made changes -
        Attachment MAPREDUCE-856-20090908.txt [ 12418893 ]
        Hide
        Hemanth Yamijala added a comment -

        Trying hudson after HADOOP-6243.

        Show
        Hemanth Yamijala added a comment - Trying hudson after HADOOP-6243 .
        Hemanth Yamijala made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12418893/MAPREDUCE-856-20090908.txt
        against trunk revision 812546.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 23 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/17/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/17/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/17/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/17/console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12418893/MAPREDUCE-856-20090908.txt against trunk revision 812546. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 23 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/17/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/17/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/17/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/17/console This message is automatically generated.
        Hide
        Hemanth Yamijala added a comment -

        I just committed this. Thanks, Vinod !

        Show
        Hemanth Yamijala added a comment - I just committed this. Thanks, Vinod !
        Hemanth Yamijala made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags [Reviewed]
        Resolution Fixed [ 1 ]
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #33 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/33/)
        . Setup secure permissions for distributed cache files. Contributed by Vinod Kumar Vavilapalli.

        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #33 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/33/ ) . Setup secure permissions for distributed cache files. Contributed by Vinod Kumar Vavilapalli.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #83 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk/83/)

        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #83 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk/83/ )
        Vinod Kumar Vavilapalli made changes -
        Release Note Fixed TaskTracker and related classes so as to set correct and most restrictive access control for DistributedCache files/archives.
         - To do this, it changed the directory structure of per-job local files on a TaskTracker to the following:
        $mapred.local.dir
           `-- taskTracker
                `-- $user
                       |- distcache
                       `-- jobcache
         - Distributed cache files/archives are now user-owned by the job-owner and the group-owned by the special group-owner of the task-controller binary. The files/archives are set most private permissions possible, and as soon as possible, immediately after the files/dirs are first localized on the TT.
         - As depicted by the new directory structure, a directory corresponding to each user is created on each TT when that particular user's first task are assigned to the corresponding TT. These user directories remain on the TT forever are not cleaned when unused, which is targeted to be fixed via MAPREDUCE-1019.
         - The distributed cache files are now accessible _only_ by the user who first localized them. Sharing of these files across users is no longer possible, but is targeted for future versions via MAPREDUCE-744.
        Hide
        Hemanth Yamijala added a comment -

        Patch for earlier version of hadoop, not for commit here.

        Show
        Hemanth Yamijala added a comment - Patch for earlier version of hadoop, not for commit here.
        Hemanth Yamijala made changes -
        Attachment MAPREDUCE-856-20090908-y20.txt [ 12431040 ]
        Tom White made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Patch Available Patch Available Open Open
        4d 2h 59m 4 Vinod Kumar Vavilapalli 08/Sep/09 10:30
        Open Open Patch Available Patch Available
        316d 16h 40m 5 Hemanth Yamijala 08/Sep/09 17:10
        Patch Available Patch Available Resolved Resolved
        5d 10h 58m 1 Hemanth Yamijala 14/Sep/09 04:08
        Resolved Resolved Closed Closed
        344d 18h 6m 1 Tom White 24/Aug/10 22:15

          People

          • Assignee:
            Vinod Kumar Vavilapalli
            Reporter:
            Arun C Murthy
          • Votes:
            0 Vote for this issue
            Watchers:
            9 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development