Hadoop Common
  1. Hadoop Common
  2. HADOOP-4446

Update Scheduling Information display in Web UI

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.19.0
    • Fix Version/s: 0.19.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Under Scheduling Information display, change label "Current Capacity" to "Guaranteed Capacity", and display only one "Guaranteed Capacity (%)" instead of displaying it separately for maps and reduces

      1. HADOOP-4446.patch
        1 kB
        Sreekanth Ramakrishnan
      2. HADOOP-4446-1.patch
        4 kB
        Sreekanth Ramakrishnan
      3. HADOOP-4446-2.patch
        4 kB
        Sreekanth Ramakrishnan
      4. HADOOP-4446-3.patch
        5 kB
        Vinod Kumar Vavilapalli

        Issue Links

          Activity

          Hide
          Karam Singh added a comment -

          On both jobtracker.jsp and jobqueue_details.jsp pages "Scheduling Inforamation" displays following information -:

          Guaranteed Capacity Maps (%) :
          Guaranteed Capacity Reduces (%) :
          Current Capacity Maps :
          Current Capacity Reduces :
          User Limit :
          Reclaim Time limit :
          Number of Running Maps :
          Number of Running Reduces :
          Number of Waiting Maps :
          Number of Waiting Reduces :
          Priority Supported :

          Here "Current Capacity Maps" and "Current Capacity Reduces" actually displays "Guaranteed Capacity" of maps and reduces. So, "Current Capacity" should "Guaranteed Capacity"

          Show
          Karam Singh added a comment - On both jobtracker.jsp and jobqueue_details.jsp pages "Scheduling Inforamation" displays following information -: Guaranteed Capacity Maps (%) : Guaranteed Capacity Reduces (%) : Current Capacity Maps : Current Capacity Reduces : User Limit : Reclaim Time limit : Number of Running Maps : Number of Running Reduces : Number of Waiting Maps : Number of Waiting Reduces : Priority Supported : Here "Current Capacity Maps" and "Current Capacity Reduces" actually displays "Guaranteed Capacity" of maps and reduces. So, "Current Capacity" should "Guaranteed Capacity"
          Hide
          Karam Singh added a comment -

          Also we should display only one Guaranteed Capacity (%) as now in configuration we specify only single guaranteed capacity instead of different guaranteed capacities for maps and reduces as it was earlier

          Show
          Karam Singh added a comment - Also we should display only one Guaranteed Capacity (%) as now in configuration we specify only single guaranteed capacity instead of different guaranteed capacities for maps and reduces as it was earlier
          Hide
          Sreekanth Ramakrishnan added a comment -

          The reason for displaying Guaranteed Capacity <Task> (%), separately is in order to be more clear for the user to view the information. The reason why we have a separate Current Capacity column is because there are can be situations in which a particular queue can have more capacity or less capacity depending on the capacities usage in the other queue. So in those cases a feedback to show how much is currently allocated, would be more clear.

          Show
          Sreekanth Ramakrishnan added a comment - The reason for displaying Guaranteed Capacity <Task> (%), separately is in order to be more clear for the user to view the information. The reason why we have a separate Current Capacity column is because there are can be situations in which a particular queue can have more capacity or less capacity depending on the capacities usage in the other queue. So in those cases a feedback to show how much is currently allocated, would be more clear.
          Hide
          Hemanth Yamijala added a comment -

          I think what Karam is pointing out is that 'Current' might mean what's currently being used, whereas what we actually mean is 'what's currently guaranteed'. So, +1 for Karam's request.

          Show
          Hemanth Yamijala added a comment - I think what Karam is pointing out is that 'Current' might mean what's currently being used, whereas what we actually mean is 'what's currently guaranteed'. So, +1 for Karam's request.
          Hide
          Sreekanth Ramakrishnan added a comment -

          Attaching the patch with changes to the toString() method of the scheduling information class which is used to display scheduling information.

          Removing an unused variable called queue in the same.

          Show
          Sreekanth Ramakrishnan added a comment - Attaching the patch with changes to the toString() method of the scheduling information class which is used to display scheduling information. Removing an unused variable called queue in the same.
          Hide
          Sreekanth Ramakrishnan added a comment -

          Attaching the output from test patch:

               [exec] -1 overall.
               [exec]
               [exec]     +1 @author.  The patch does not contain any @author tags.
               [exec]
               [exec]     -1 tests included.  The patch doesn't appear to include any new or modified tests.
               [exec]                         Please justify why no tests are needed for this patch.
               [exec]
               [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
               [exec]
               [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
               [exec]
               [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
               [exec]
               [exec]     +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
               [exec]
          

          This patch does not require a test case because it just modifies the toString() method of scheduling information.

          Show
          Sreekanth Ramakrishnan added a comment - Attaching the output from test patch: [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] -1 tests included. The patch doesn't appear to include any new or modified tests. [exec] Please justify why no tests are needed for this patch. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity. [exec] This patch does not require a test case because it just modifies the toString() method of scheduling information.
          Hide
          Vivek Ratan added a comment -

          Just to make this clear: Queues have guaranteed capacities, but their number of running tasks may be higher or lower. There's no separate concept of 'current capacity', or any other capacity besides Guaranteed Capacity. Sreekanth, maybe your patch already does this, but I'd recommend that we replace the strings "Current Capacity Maps" and "Current Capacity Reduces" with "Guaranteed Capacity Maps" and "Guaranteed Capacity Reduces". And the value of these fields should be the absolute numbers of the percentage values, i.e., 'Guaranteed Capacity Maps' = 'Guaranteed Capacity Maps (%)' * Cluster Capacity.

          Show
          Vivek Ratan added a comment - Just to make this clear: Queues have guaranteed capacities, but their number of running tasks may be higher or lower. There's no separate concept of 'current capacity', or any other capacity besides Guaranteed Capacity. Sreekanth, maybe your patch already does this, but I'd recommend that we replace the strings "Current Capacity Maps" and "Current Capacity Reduces" with "Guaranteed Capacity Maps" and "Guaranteed Capacity Reduces". And the value of these fields should be the absolute numbers of the percentage values, i.e., 'Guaranteed Capacity Maps' = 'Guaranteed Capacity Maps (%)' * Cluster Capacity.
          Hide
          Sreekanth Ramakrishnan added a comment -

          I have changed the strings to read as "Guaranteed Capacity <Task>" and has a single Guaranteed Capacity (%) column now being displayed.

          Show
          Sreekanth Ramakrishnan added a comment - I have changed the strings to read as "Guaranteed Capacity <Task>" and has a single Guaranteed Capacity (%) column now being displayed.
          Hide
          Hemanth Yamijala added a comment -

          I see we don't have a test case for verifying what we show in the UI. The patch for this bug would be a great place to add it, along with getting an overall +1 blessing from ant test-patch.

          Show
          Hemanth Yamijala added a comment - I see we don't have a test case for verifying what we show in the UI. The patch for this bug would be a great place to add it, along with getting an overall +1 blessing from ant test-patch.
          Hide
          Sreekanth Ramakrishnan added a comment -

          Attaching patch adding a test case to check the scheduling information which is returned by the string.

          Show
          Sreekanth Ramakrishnan added a comment - Attaching patch adding a test case to check the scheduling information which is returned by the string.
          Hide
          Sreekanth Ramakrishnan added a comment -

          Attaching the result of ant test-patch:

               [exec] +1 overall.  
               [exec] 
               [exec]     +1 @author.  The patch does not contain any @author tags.
               [exec] 
               [exec]     +1 tests included.  The patch appears to include 3 new or modified tests.
               [exec] 
               [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
               [exec] 
               [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
               [exec] 
               [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
               [exec] 
               [exec]     +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
               [exec] 
          
          Show
          Sreekanth Ramakrishnan added a comment - Attaching the result of ant test-patch: [exec] +1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 3 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity. [exec]
          Hide
          Vinod Kumar Vavilapalli added a comment -

          Minor comments about the test-case.

          • The name should be testSchedulingInformation.
          • We don't need to test explicit omit of "Current Capacity *". We could just explode the Scheduling info string by new-line to get each key-val pair and test that each key has correct value and that only there are only as many pairs as needed.
          Show
          Vinod Kumar Vavilapalli added a comment - Minor comments about the test-case. The name should be testSchedulingInformation. We don't need to test explicit omit of "Current Capacity *". We could just explode the Scheduling info string by new-line to get each key-val pair and test that each key has correct value and that only there are only as many pairs as needed.
          Hide
          Sreekanth Ramakrishnan added a comment -

          Attaching patch with comments incorporated.

          Show
          Sreekanth Ramakrishnan added a comment - Attaching patch with comments incorporated.
          Hide
          Sreekanth Ramakrishnan added a comment -

          Running patch thro' hudson.

          Show
          Sreekanth Ramakrishnan added a comment - Running patch thro' hudson.
          Hide
          Vinod Kumar Vavilapalli added a comment -

          Attaching a new patch with slight modifications to the test so that all the information present in scheduler information is tested.

          Show
          Vinod Kumar Vavilapalli added a comment - Attaching a new patch with slight modifications to the test so that all the information present in scheduler information is tested.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12393152/HADOOP-4446-3.patch
          against trunk revision 709609.

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

          +1 tests included. The patch appears to include 3 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 Eclipse classpath. The patch retains Eclipse classpath integrity.

          +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/Hadoop-Patch/3514/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3514/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3514/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3514/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/12393152/HADOOP-4446-3.patch against trunk revision 709609. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 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 Eclipse classpath. The patch retains Eclipse classpath integrity. +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/Hadoop-Patch/3514/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3514/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3514/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3514/console This message is automatically generated.
          Hide
          Hemanth Yamijala added a comment -

          I just committed this. Thanks, Sreekanth (and Vinod for the updated test case).

          Show
          Hemanth Yamijala added a comment - I just committed this. Thanks, Sreekanth (and Vinod for the updated test case).

            People

            • Assignee:
              Sreekanth Ramakrishnan
              Reporter:
              Karam Singh
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development