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

MR1 FairScheduler use of custom weight adjuster is not thread safe for comparisons

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.1
    • Fix Version/s: 1.3.0
    • Component/s: scheduler
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      When comparing JobSchedulables one of the factors is the weight. If someone uses a custom weight adjuster, that may be called multiple times during a sort causing different values to return. That causes a failure in sorting because the weight may change during the sort.

      This reproes as

      java.io.IOException: java.lang.IllegalArgumentException: Comparison method violates its general contract!
      at java.util.TimSort.mergeHi(TimSort.java:868)
      at java.util.TimSort.mergeAt(TimSort.java:485)
      at java.util.TimSort.mergeCollapse(TimSort.java:410)
      at java.util.TimSort.sort(TimSort.java:214)
      at java.util.TimSort.sort(TimSort.java:173)
      at java.util.Arrays.sort(Arrays.java:659)
      at java.util.Collections.sort(Collections.java:217)
      at org.apache.hadoop.mapred.PoolSchedulable.assignTask(PoolSchedulable.java:163)
      at org.apache.hadoop.mapred.FairScheduler.assignTasks(FairScheduler.java:499)
      at org.apache.hadoop.mapred.JobTracker.heartbeat(JobTracker.java:2961)
      
      1. mr-5966-3.patch
        8 kB
        Karthik Kambatla
      2. MAPREDUCE-5966.002.patch
        9 kB
        Anubhav Dhoot
      3. MAPREDUCE-5966.001.patch
        8 kB
        Anubhav Dhoot

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        7d 17m 1 Anubhav Dhoot 18/Jul/14 01:39
        Patch Available Patch Available Resolved Resolved
        8d 19h 14m 1 Karthik Kambatla (Inactive) 26/Jul/14 20:54
        Karthik Kambatla (Inactive) made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags Reviewed [ 10343 ]
        Fix Version/s 1.3.0 [ 12324153 ]
        Resolution Fixed [ 1 ]
        Hide
        Karthik Kambatla (Inactive) added a comment - - edited

        Thanks for the patch, Anubhav. Just committed to branch-1.

        Show
        Karthik Kambatla (Inactive) added a comment - - edited Thanks for the patch, Anubhav. Just committed to branch-1.
        Hide
        Anubhav Dhoot added a comment -

        Changes look good to me

        Show
        Anubhav Dhoot added a comment - Changes look good to me
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12657957/mr-5966-3.patch
        against trunk revision .

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

        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/4770//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/12657957/mr-5966-3.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/4770//console This message is automatically generated.
        Karthik Kambatla (Inactive) made changes -
        Attachment mr-5966-3.patch [ 12657957 ]
        Hide
        Karthik Kambatla (Inactive) added a comment -

        The patch looks good to me. Uploading same patch with some minor cosmetic changes.

        Anubhav Dhoot - let me know if you agree with those changes.

        Show
        Karthik Kambatla (Inactive) added a comment - The patch looks good to me. Uploading same patch with some minor cosmetic changes. Anubhav Dhoot - let me know if you agree with those changes.
        Hide
        Hadoop QA added a comment -

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

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

        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/4763//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/12657405/MAPREDUCE-5966.002.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/4763//console This message is automatically generated.
        Hide
        Anubhav Dhoot added a comment -

        >Also, don't see the need for numRacks and numNodesPerRack.
        Its used both for setupCluster and calculating TaskTrackers

        Show
        Anubhav Dhoot added a comment - >Also, don't see the need for numRacks and numNodesPerRack. Its used both for setupCluster and calculating TaskTrackers
        Anubhav Dhoot made changes -
        Attachment MAPREDUCE-5966.002.patch [ 12657405 ]
        Hide
        Anubhav Dhoot added a comment -

        Attaching patch that addresses feedback

        Show
        Anubhav Dhoot added a comment - Attaching patch that addresses feedback
        Hide
        Karthik Kambatla (Inactive) added a comment -

        Looks like this patch doesn't apply anymore, may be due MR-5979. Can you please update it?

        Also, I have the following minor comments:

        1. Reword the following comment to say "Update demands and weights of jobs and pools"
          "+      // Update demands of jobs and pools and update weights
          
        2. In the test case, I don't think Math.max is required anymore.
          +
          +        // Until MAPREDUCE-5966 gets fixed we cannot have zero weight set
          +        return Math.max(curWeight * random, 0.001);
          
        3. We should be able to fit the following in two lines with throws in the line after the method name?
          +  public void testJobSchedulableSortingWithCustomWeightAdjuster() throws
          +      IOException,
          +      InterruptedException {
          
        4. Can we make all these variables final and capital letters. Also, don't see the need for numRacks and numNodesPerRack.
          +    final int iterations = 100;
          +    int jobCount = 100;
          +    int numRacks = 100;
          +    int numNodesPerRack = 2;
          +    final int totalTaskTrackers = numNodesPerRack * numRacks;
          
        5. We should probably use pure camel-caps for this variable - randomTtid
        Show
        Karthik Kambatla (Inactive) added a comment - Looks like this patch doesn't apply anymore, may be due MR-5979. Can you please update it? Also, I have the following minor comments: Reword the following comment to say "Update demands and weights of jobs and pools" "+ // Update demands of jobs and pools and update weights In the test case, I don't think Math.max is required anymore. + + // Until MAPREDUCE-5966 gets fixed we cannot have zero weight set + return Math .max(curWeight * random, 0.001); We should be able to fit the following in two lines with throws in the line after the method name? + public void testJobSchedulableSortingWithCustomWeightAdjuster() throws + IOException, + InterruptedException { Can we make all these variables final and capital letters. Also, don't see the need for numRacks and numNodesPerRack. + final int iterations = 100; + int jobCount = 100; + int numRacks = 100; + int numNodesPerRack = 2; + final int totalTaskTrackers = numNodesPerRack * numRacks; We should probably use pure camel-caps for this variable - randomTtid
        Hide
        Hadoop QA added a comment -

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

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

        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/4754//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/12656402/MAPREDUCE-5966.001.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/4754//console This message is automatically generated.
        Anubhav Dhoot made changes -
        Attachment MAPREDUCE-5966.001.patch [ 12656402 ]
        Hide
        Anubhav Dhoot added a comment -

        Testing: Verified running ant test under src/contrib/fairscheduler passes

        Show
        Anubhav Dhoot added a comment - Testing: Verified running ant test under src/contrib/fairscheduler passes
        Anubhav Dhoot made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Anubhav Dhoot added a comment -

        Submitting patch that does not allow changes in the weight when sorting starts. All updates to weight are done at the same time and in the same way demands and other changes are updated.

        Show
        Anubhav Dhoot added a comment - Submitting patch that does not allow changes in the weight when sorting starts. All updates to weight are done at the same time and in the same way demands and other changes are updated.
        Karthik Kambatla (Inactive) made changes -
        Affects Version/s 1.2.1 [ 12324149 ]
        Target Version/s 1.3.0 [ 12324153 ]
        Anubhav Dhoot made changes -
        Component/s scheduler [ 12315345 ]
        Anubhav Dhoot made changes -
        Field Original Value New Value
        Assignee Anubhav Dhoot [ adhoot ]
        Anubhav Dhoot created issue -

          People

          • Assignee:
            Anubhav Dhoot
            Reporter:
            Anubhav Dhoot
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development