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

Generate configuration dump for hierarchial queue configuration

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.21.0
    • Fix Version/s: 0.21.0
    • Component/s: jobtracker
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Generate configuration dump for hierarchial queue configuration

      1. MAPREDUCE-953-2.patch
        12 kB
        V.V.Chaitanya Krishna
      2. MAPREDUCE-953-1.patch
        14 kB
        V.V.Chaitanya Krishna

        Activity

        Hide
        V.V.Chaitanya Krishna added a comment -

        The proposal of the JSONN string format is as follows:

        {"queues":[
        {"name":<queue_name>,"state":<queue_state>,"submit_job":<submit-job acls>,
        "administer_jobs":<administer-jobs-acls>,"properties"<properties>,"children":[

        {"name":<child queue name>,"state":<child_queue_state>, "submit_job":<child_submit-job acls>, "administer_jobs":<child_administer-jobs-acls>,"properties":<child_properties>,"children":[<child queues>] }

        ]
        }
        ] }

        Note: The <children queues> will be again in the same format as its parent is. So, it is a tree-like structure.

        Show
        V.V.Chaitanya Krishna added a comment - The proposal of the JSONN string format is as follows: {"queues":[ {"name":<queue_name>,"state":<queue_state>,"submit_job":<submit-job acls>, "administer_jobs":<administer-jobs-acls>,"properties"<properties>,"children":[ {"name":<child queue name>,"state":<child_queue_state>, "submit_job":<child_submit-job acls>, "administer_jobs":<child_administer-jobs-acls>,"properties":<child_properties>,"children":[<child queues>] } ] } ] } Note: The <children queues> will be again in the same format as its parent is. So, it is a tree-like structure.
        Hide
        V.V.Chaitanya Krishna added a comment -

        Uploading patch taking the above mentioned format.
        Proper test cases are also included.

        Show
        V.V.Chaitanya Krishna added a comment - Uploading patch taking the above mentioned format. Proper test cases are also included.
        Hide
        Hemanth Yamijala added a comment -
        • QueueManager.dumpConfiguration currently assumes that the new configuration is being used because it will call getQueueConfigurationParser with a null configuration, that loads the new QueueConfigurationParser. It should take the Configuration instance from JobTracker, and check if the queues are configured there. If yes, it should return silently as the config would have been dumped in the JT itself. Otherwise, it can use a QueueConfigurationParser
        • We are not dumping whether acls are enabled or not.
        • To get the ACLs from the ACLs map, you should use the API toFullPropertyName.
        • We have no way to separate ACL user names and groups - it will be difficult for admins to separate them, Why not use the existing format itself: u1,u2 g1,g2 - will
          having a comma be a problem for json ?
        • At any rate the code to build the list of users and groups should be pulled into a separate utility API and called for both the ACLs.
        • Use acl_submit_job and acl_administer_jobs as keys.
        Show
        Hemanth Yamijala added a comment - QueueManager.dumpConfiguration currently assumes that the new configuration is being used because it will call getQueueConfigurationParser with a null configuration, that loads the new QueueConfigurationParser. It should take the Configuration instance from JobTracker, and check if the queues are configured there. If yes, it should return silently as the config would have been dumped in the JT itself. Otherwise, it can use a QueueConfigurationParser We are not dumping whether acls are enabled or not. To get the ACLs from the ACLs map, you should use the API toFullPropertyName. We have no way to separate ACL user names and groups - it will be difficult for admins to separate them, Why not use the existing format itself: u1,u2 g1,g2 - will having a comma be a problem for json ? At any rate the code to build the list of users and groups should be pulled into a separate utility API and called for both the ACLs. Use acl_submit_job and acl_administer_jobs as keys.
        Hide
        Hemanth Yamijala added a comment -

        Looked at the test case also. One comment I have is that we should verify the ACLs, properties and children in a different manner I think. Because right now, we are constructing the ACLs in the test case using similar code as in the actual code. This leads to a chance that we could make the same mistake at both places. I think it may be OK to hardcode what we are verifying, rather than writing generically. For e.g. we know what the queue configuration is. We know which queues have what ACLs and what properties, we can hardcode that verification. That way, the test will be simpler and it will be correct.

        For e.g., now because both test and code make the same mistake about the ACL key, the test doesn't fail, but it doesn't catch the bug as well.

        Show
        Hemanth Yamijala added a comment - Looked at the test case also. One comment I have is that we should verify the ACLs, properties and children in a different manner I think. Because right now, we are constructing the ACLs in the test case using similar code as in the actual code. This leads to a chance that we could make the same mistake at both places. I think it may be OK to hardcode what we are verifying, rather than writing generically. For e.g. we know what the queue configuration is. We know which queues have what ACLs and what properties, we can hardcode that verification. That way, the test will be simpler and it will be correct. For e.g., now because both test and code make the same mistake about the ACL key, the test doesn't fail, but it doesn't catch the bug as well.
        Hide
        V.V.Chaitanya Krishna added a comment -

        Uploading the patch with the above suggested modifications done.
        In order to dump acls_enabled, the json format is changed to the following:

        {"acls_enabled":true, "queues":[
        {"name":<queue_name>,"state":<queue_state>,"submit_job":<submit-job acls>,
        "administer_jobs":<administer-jobs-acls>,"properties"<properties>,"children":[

        {"name":<child queue name>,"state":<child_queue_state>, "submit_job":<child_submit-job acls>, "administer_jobs":<child_administer-jobs-acls>,"properties":<child_properties>,"children":[<child queues>] }

        ]
        }
        ] }

        Show
        V.V.Chaitanya Krishna added a comment - Uploading the patch with the above suggested modifications done. In order to dump acls_enabled, the json format is changed to the following: {"acls_enabled":true, "queues":[ {"name":<queue_name>,"state":<queue_state>,"submit_job":<submit-job acls>, "administer_jobs":<administer-jobs-acls>,"properties"<properties>,"children":[ {"name":<child queue name>,"state":<child_queue_state>, "submit_job":<child_submit-job acls>, "administer_jobs":<child_administer-jobs-acls>,"properties":<child_properties>,"children":[<child queues>] } ] } ] }
        Hide
        Hemanth Yamijala added a comment -

        This looks good to me. Passing through Hudson.

        Show
        Hemanth Yamijala added a comment - This looks good to me. Passing through Hudson.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12419909/MAPREDUCE-953-2.patch
        against trunk revision 816439.

        +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 release audit. The applied patch does not increase the total number of release audit warnings.

        +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/Mapreduce-Patch-h3.grid.sp2.yahoo.net/47/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/47/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/47/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/47/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/12419909/MAPREDUCE-953-2.patch against trunk revision 816439. +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 release audit. The applied patch does not increase the total number of release audit warnings. +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/Mapreduce-Patch-h3.grid.sp2.yahoo.net/47/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/47/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/47/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/47/console This message is automatically generated.
        Hide
        Hemanth Yamijala added a comment -

        I just committed this. Thanks, Chaitanya !

        Show
        Hemanth Yamijala added a comment - I just committed this. Thanks, Chaitanya !
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #49 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/49/)
        . Fix QueueManager to dump queue configuration in JSON format. Contributed by V.V. Chaitanya Krishna.

        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #49 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/49/ ) . Fix QueueManager to dump queue configuration in JSON format. Contributed by V.V. Chaitanya Krishna.

          People

          • Assignee:
            V.V.Chaitanya Krishna
            Reporter:
            rahul k singh
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development