Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-5211

Reducer intermediate files can collide during merge

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.23.7
    • Fix Version/s: 0.23.8
    • Component/s: mrv2
    • Labels:

      Description

      The OnDiskMerger.merge method constructs an output path that is not unique to a reduce attempt, and as a result can result in a file collision with other reducers from the same app that are running on the same node. In addition the name of the output file is based on MapOutput.toString which may not be unique in light of multi-pass merges on disk since the mapId will be null and the basename ends up as "MapOutput(null, DISK)"

        Issue Links

          Activity

          Hide
          Jason Lowe added a comment -

          This bug appears to have been introduced by MAPREDUCE-3685, as previously the code was deriving an output path from a full path to an on-disk input file but now is simply using toString() on a MapOutput which is not nearly unique enough.

          Show
          Jason Lowe added a comment - This bug appears to have been introduced by MAPREDUCE-3685 , as previously the code was deriving an output path from a full path to an on-disk input file but now is simply using toString() on a MapOutput which is not nearly unique enough.
          Hide
          Jason Lowe added a comment -

          This is not an issue on branch-2 or trunk because it was fixed (accidentally?) by MAPREDUCE-4808.

          Show
          Jason Lowe added a comment - This is not an issue on branch-2 or trunk because it was fixed (accidentally?) by MAPREDUCE-4808 .
          Hide
          Jason Lowe added a comment -

          Patch for branch-0.23 that adds the reduce task attempt ID to the output path along with an increasing sequence number to keep the output files from colliding.

          No unit test, but manually tested to verify output paths for on disk merges are emitted properly.

          Show
          Jason Lowe added a comment - Patch for branch-0.23 that adds the reduce task attempt ID to the output path along with an increasing sequence number to keep the output files from colliding. No unit test, but manually tested to verify output paths for on disk merges are emitted properly.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12581974/MAPREDUCE-5211.branch-0.23.patch
          against trunk revision .

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

          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3574//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/12581974/MAPREDUCE-5211.branch-0.23.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3574//console This message is automatically generated.
          Hide
          Siddharth Seth added a comment -

          Jason, the 0.23 patch looks ok. Maybe we should add a utility method to generate intermediate output file names.

          I think a variant of this does exist in trunk as well. The OnDiskMerger it's first inputPath as the outputPath - which means it could end up overwriting this file in a multi-stage merge ?

          Show
          Siddharth Seth added a comment - Jason, the 0.23 patch looks ok. Maybe we should add a utility method to generate intermediate output file names. I think a variant of this does exist in trunk as well. The OnDiskMerger it's first inputPath as the outputPath - which means it could end up overwriting this file in a multi-stage merge ?
          Hide
          Arun C Murthy added a comment -

          +1 for the branch-0.23 patch.

          Looks like this existed forever in trunk and got accidentally fixed by MAPREDUCE-2264 which introduced CompressAwarePath which extends Path i.e. with a correct impl of toString - hence the bug in MapOutput.toString got fixed accidentally.

          However, the fix in trunk is truly accidental, and we'll get paths which have the suffix ".merge" for each level of merge etc. repeated.

          So, we should apply a similar fix to trunk too.

          Show
          Arun C Murthy added a comment - +1 for the branch-0.23 patch. Looks like this existed forever in trunk and got accidentally fixed by MAPREDUCE-2264 which introduced CompressAwarePath which extends Path i.e. with a correct impl of toString - hence the bug in MapOutput.toString got fixed accidentally. However, the fix in trunk is truly accidental, and we'll get paths which have the suffix ".merge" for each level of merge etc. repeated. So, we should apply a similar fix to trunk too.
          Hide
          Jason Lowe added a comment -

          I believe 1.x also has a similar behavior where it will concatenate the absolute path of the first input path to the temporary directory. Therefore if we have multi-level merges then the paths will be increasingly lengthy as it keeps tacking on the temporary directory to the input path.

          Show
          Jason Lowe added a comment - I believe 1.x also has a similar behavior where it will concatenate the absolute path of the first input path to the temporary directory. Therefore if we have multi-level merges then the paths will be increasingly lengthy as it keeps tacking on the temporary directory to the input path.
          Hide
          Jason Lowe added a comment -

          Thanks for the reviews. I committed this to branch-0.23.

          I do not believe the collision in branch-2/trunk is possible since it concatenates an absolute path of a source file to a working directory specific to the reduce attempt to build a destination path. The pathnames can become very long, but I don't think they will collide. If we want to change that behavior, we can handle it in another JIRA.

          Show
          Jason Lowe added a comment - Thanks for the reviews. I committed this to branch-0.23. I do not believe the collision in branch-2/trunk is possible since it concatenates an absolute path of a source file to a working directory specific to the reduce attempt to build a destination path. The pathnames can become very long, but I don't think they will collide. If we want to change that behavior, we can handle it in another JIRA.
          Hide
          Konstantin Boudnik added a comment -

          This needs to go into the next stabilization release of 2.0.4.2 as well. Adding the label to that effect and re-opening the ticket.

          Show
          Konstantin Boudnik added a comment - This needs to go into the next stabilization release of 2.0.4.2 as well. Adding the label to that effect and re-opening the ticket.
          Hide
          Jason Lowe added a comment -

          Is this actually a problem in 2.0.4? I believe after MAPREDUCE-2264, which is in 2.0.3, MapOutput objects are not used to generate the pathnames and the problem does not occur. The pathnames can get very long, since they concatenate two absolute paths to form the new path, but I didn't think it was possible for two paths to collide for the same reducer. Also the reduce attempt ID should appear at least once in the pathname, so it should also be impossible for paths to collide between reducers.

          Show
          Jason Lowe added a comment - Is this actually a problem in 2.0.4? I believe after MAPREDUCE-2264 , which is in 2.0.3, MapOutput objects are not used to generate the pathnames and the problem does not occur. The pathnames can get very long, since they concatenate two absolute paths to form the new path, but I didn't think it was possible for two paths to collide for the same reducer. Also the reduce attempt ID should appear at least once in the pathname, so it should also be impossible for paths to collide between reducers.
          Hide
          Arun C Murthy added a comment -

          Not a problem in branch-2 per Jason Lowe comment.

          Show
          Arun C Murthy added a comment - Not a problem in branch-2 per Jason Lowe comment.

            People

            • Assignee:
              Jason Lowe
              Reporter:
              Jason Lowe
            • Votes:
              0 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development