Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.0.0-alpha
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      Let's rename DefaultResourceCalculator to something like MemoryResourceCalculator or SingleResourceCalculator. The default resource calculator is the one specified by yarn.scheduler.capacity.resource-calculator in yarn-default.xml (which may change). We can do this compatibly now since YARN-2 hasn't been released yet, but changing this later will be a pain if we ever make a different resource calculator the default (or DefaultResourceCalculator won't actually be the default, which is weird).

      1. yarn-340.txt
        16 kB
        Eli Collins
      2. yarn-340.txt
        18 kB
        Eli Collins

        Activity

        Hide
        Eli Collins added a comment -

        Patch attached. (Used svn mv for DefaultResourceCalculator to maintain history, so this patch shouldn't be checked in verbatim)

        Show
        Eli Collins added a comment - Patch attached. (Used svn mv for DefaultResourceCalculator to maintain history, so this patch shouldn't be checked in verbatim)
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12564791/yarn-340.txt
        against trunk revision .

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

        Console output: https://builds.apache.org/job/PreCommit-YARN-Build/347//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/12564791/yarn-340.txt against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-YARN-Build/347//console This message is automatically generated.
        Hide
        Eli Collins added a comment -

        Looks like svn diff after doing a mv and edit didn't generate a diff that test-patch could apply, this one should work.

        Show
        Eli Collins added a comment - Looks like svn diff after doing a mv and edit didn't generate a diff that test-patch could apply, this one should work.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12564795/yarn-340.txt
        against trunk revision .

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

        +1 tests included. The patch appears to include 4 new or modified test files.

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

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

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        -1 release audit. The applied patch generated 2 release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager.

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

        Test results: https://builds.apache.org/job/PreCommit-YARN-Build/348//testReport/
        Release audit warnings: https://builds.apache.org/job/PreCommit-YARN-Build/348//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt
        Console output: https://builds.apache.org/job/PreCommit-YARN-Build/348//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/12564795/yarn-340.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. -1 release audit . The applied patch generated 2 release audit warnings. +1 core tests . The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/348//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-YARN-Build/348//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Console output: https://builds.apache.org/job/PreCommit-YARN-Build/348//console This message is automatically generated.
        Hide
        Eli Collins added a comment -

        Release audit warnings are unrelated (YARN-341)

        Show
        Eli Collins added a comment - Release audit warnings are unrelated ( YARN-341 )
        Hide
        Arun C Murthy added a comment -

        Eli - I was specifically asked to not call it MemoryResourceCalculator during reviews in YARN-2 (the fear was that we'd proliferate MemoryCalculator MemoryCPUCalculator etc.).

        Show
        Arun C Murthy added a comment - Eli - I was specifically asked to not call it MemoryResourceCalculator during reviews in YARN-2 (the fear was that we'd proliferate MemoryCalculator MemoryCPUCalculator etc.).
        Hide
        Eli Collins added a comment - - edited

        Yea, that's why I suggested SingleResourceCalculator as well, what do you think of using it? I actually think your original name was better (and don't think we'll end up with that many permutations in practice which is why I went with MemoryResourceCalculator) but if people are concerned with that at least SingleResourceCalculator means we won't proliferate as many but we'll still be able to change the default in the future without renaming classes or having the DefaultResourceCalculator not actually be the default.

        Show
        Eli Collins added a comment - - edited Yea, that's why I suggested SingleResourceCalculator as well, what do you think of using it? I actually think your original name was better (and don't think we'll end up with that many permutations in practice which is why I went with MemoryResourceCalculator) but if people are concerned with that at least SingleResourceCalculator means we won't proliferate as many but we'll still be able to change the default in the future without renaming classes or having the DefaultResourceCalculator not actually be the default.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12564795/yarn-340.txt
        against trunk revision .

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

        +1 tests included. The patch appears to include 4 new or modified test files.

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

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

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

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

        +1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager.

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

        Test results: https://builds.apache.org/job/PreCommit-YARN-Build/349//testReport/
        Console output: https://builds.apache.org/job/PreCommit-YARN-Build/349//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/12564795/yarn-340.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/349//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/349//console This message is automatically generated.
        Hide
        Karthik Kambatla (Inactive) added a comment -

        +1 on the SingleResourceCalculator idea.

        Show
        Karthik Kambatla (Inactive) added a comment - +1 on the SingleResourceCalculator idea.
        Hide
        Vinod Kumar Vavilapalli added a comment -

        "Single" is equally confusing ("which one is it?"). How about MemoryBasedResourceCalculator? Anything more (like MemoryCPUResrouceCalculator), we should religiously invent new names (like we did for schedulers).

        Show
        Vinod Kumar Vavilapalli added a comment - "Single" is equally confusing ("which one is it?"). How about MemoryBasedResourceCalculator? Anything more (like MemoryCPUResrouceCalculator), we should religiously invent new names (like we did for schedulers).
        Hide
        Eli Collins added a comment -

        I like MemoryResourceCalculator (or MemoryBasedResourceCalculator) better as well, that was my first choice, and is what the attached patch uses. Arun went with that on YARN-2 but people objected, if people are OK with it now then let's go with it. I don't think proliferation of many class names will be an issue.

        Show
        Eli Collins added a comment - I like MemoryResourceCalculator (or MemoryBasedResourceCalculator) better as well, that was my first choice, and is what the attached patch uses. Arun went with that on YARN-2 but people objected, if people are OK with it now then let's go with it. I don't think proliferation of many class names will be an issue.
        Hide
        Arun C Murthy added a comment -

        Cleaning up stale PA patches.

        Show
        Arun C Murthy added a comment - Cleaning up stale PA patches.

          People

          • Assignee:
            Eli Collins
            Reporter:
            Eli Collins
          • Votes:
            0 Vote for this issue
            Watchers:
            10 Start watching this issue

            Dates

            • Created:
              Updated:

              Development