Hadoop Common
  1. Hadoop Common
  2. HADOOP-1077

Race condition in fetching map outputs (might lead to hung reduces)

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.12.1
    • Component/s: None
    • Labels:
      None

      Description

      Sometimes when a map task is lost while the map-output fetch is happening from the TT for that task, and the lost map has successfully executed on some other node, the event for that successful execution is lost at the fetching TT. The fetching TT might eventually fail to fetch the output for the lost task, but then since the event for the new run of the lost map might also have been lost, the fetching TT might hang.

      This "hung" problem was discovered while working on HADOOP-1060.

      1. 1077.2.patch
        6 kB
        Arun C Murthy
      2. 1077.patch
        7 kB
        Devaraj Das

        Issue Links

          Activity

          Hide
          Devaraj Das added a comment -

          A clarification (the description should read as follows):
          Sometimes when a map task is lost while the map-output fetch is happening from the TT for that task, and the lost map has, in the meantime, successfully executed on some other node, the event for that successful execution is lost at the fetching TT. The fetching TT might eventually fail to fetch the output for the lost task, but then since the event for the new run of the lost map might also have been lost, the fetching TT might hang.

          Show
          Devaraj Das added a comment - A clarification (the description should read as follows): Sometimes when a map task is lost while the map-output fetch is happening from the TT for that task, and the lost map has, in the meantime , successfully executed on some other node, the event for that successful execution is lost at the fetching TT. The fetching TT might eventually fail to fetch the output for the lost task, but then since the event for the new run of the lost map might also have been lost, the fetching TT might hang.
          Hide
          Devaraj Das added a comment -

          This patch does the following:
          1) Adds a new hashmap to maintain the fetches-in-progress. This is used to track fetches and to take of the problem where we receive a new map output location for a given mapId while we are fetching (and will probably fail) the output for that same mapId from some other location.
          2) An entry for a mapId is made in the fetchInProgress map as soon as we receive a map output location from the JobTracker if the neededOutputs has the mapId. The entry for the mapId is not deleted until we have successfully copied the output.
          3) During the time the output is copied, it might so happen that the TT is lost and we come to know about it after a while (after connect times out, etc.). In the meanwhile, if another execution of the lost task happened somewhere, we get the event for that and schedule that fetch as well. The output of first successful copier is considered as valid and the other becomes obsolete. In the existing code, this second event would be lost since we depended only on the neededOutputs list (from where entries are removed as soon as the fetches are scheduled). Now an additional check is done to see whether the mapId exists in fetchesInProgress.

          Show
          Devaraj Das added a comment - This patch does the following: 1) Adds a new hashmap to maintain the fetches-in-progress. This is used to track fetches and to take of the problem where we receive a new map output location for a given mapId while we are fetching (and will probably fail) the output for that same mapId from some other location. 2) An entry for a mapId is made in the fetchInProgress map as soon as we receive a map output location from the JobTracker if the neededOutputs has the mapId. The entry for the mapId is not deleted until we have successfully copied the output. 3) During the time the output is copied, it might so happen that the TT is lost and we come to know about it after a while (after connect times out, etc.). In the meanwhile, if another execution of the lost task happened somewhere, we get the event for that and schedule that fetch as well. The output of first successful copier is considered as valid and the other becomes obsolete. In the existing code, this second event would be lost since we depended only on the neededOutputs list (from where entries are removed as soon as the fetches are scheduled). Now an additional check is done to see whether the mapId exists in fetchesInProgress.
          Hide
          David Bowen added a comment -

          It is nice to see a patch with such good comments!

          At the risk of being a coding-style bore, here are a couple of very minor suggestions: (1) long synchronized blocks are a bit hard to read given the two-space indentation style - it may be preferable to break them out into separate methods; (2) some may disagree, but I see no need to write method arguments like "new Integer(loc.getMapId())" when you can now write just "loc.getMapId()" and the compiler will automatically do the conversion.

          Show
          David Bowen added a comment - It is nice to see a patch with such good comments! At the risk of being a coding-style bore, here are a couple of very minor suggestions: (1) long synchronized blocks are a bit hard to read given the two-space indentation style - it may be preferable to break them out into separate methods; (2) some may disagree, but I see no need to write method arguments like "new Integer(loc.getMapId())" when you can now write just "loc.getMapId()" and the compiler will automatically do the conversion.
          Hide
          Arun C Murthy added a comment -

          Attaching Devaraj's patch since he is asleep by now...

          Show
          Arun C Murthy added a comment - Attaching Devaraj's patch since he is asleep by now...
          Hide
          Doug Cutting added a comment -

          I just committed this. Thanks, Devaraj!

          Show
          Doug Cutting added a comment - I just committed this. Thanks, Devaraj!

            People

            • Assignee:
              Devaraj Das
              Reporter:
              Devaraj Das
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development