Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-5660

Wrong audit constants are used in Get/Put of priority in RMWebService

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha2
    • Component/s: resourcemanager
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      In methods RMWebService#getAppPriority and RMWebService#updateApplicationPriority uses wrong audit constants for RMAuditLogger. It should be AuditConstants.UPDATE_APP_PRIORITY

      RMApp app = null;
          try {
            app = getRMAppForAppId(appId);
          } catch (NotFoundException e) {
            RMAuditLogger.logFailure(userName, AuditConstants.KILL_APP_REQUEST,
                "UNKNOWN", "RMWebService",
                "Trying to move an absent application " + appId);
            throw e;
          }
      
      1. YARN-5660.patch
        3 kB
        Rohith Sharma K S
      2. YARN-5660.1.patch
        5 kB
        Rohith Sharma K S

        Activity

        Hide
        Naganarasimha Naganarasimha G R added a comment - - edited

        Rohith Sharma K S,
        Adding to it i think its not required to add audit logs for get priority failure & audit log failure message "Trying to move an absent application " needs to be modified appropriately ...

        Show
        Naganarasimha Naganarasimha G R added a comment - - edited Rohith Sharma K S , Adding to it i think its not required to add audit logs for get priority failure & audit log failure message "Trying to move an absent application " needs to be modified appropriately ...
        Hide
        vvasudev Varun Vasudev added a comment -

        My apologies for missing this when I did the review.

        Show
        vvasudev Varun Vasudev added a comment - My apologies for missing this when I did the review.
        Hide
        sunilg Sunil G added a comment -

        Hi Naganarasimha Garla
        We need audit log for failures too, correct? If application is not found for given app id, i think we can audit log the same for tracking purpose. Do you see some issues in this line.

        Show
        sunilg Sunil G added a comment - Hi Naganarasimha Garla We need audit log for failures too, correct? If application is not found for given app id, i think we can audit log the same for tracking purpose. Do you see some issues in this line.
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Well not sure why for all the REST queries are doing audit logs for failures when the app id is not specified. So would be ok for doing this too,
        IMO is its a invalid input from user end and also not an update activity hence not sure why audit log is required.

        other than that latest patch looks fine except for the nit that
        RMAuditLogger.UPDATE_APP_PRIORITY can be used in RMWebServices.updateApplicationPriority method instead of new constant APP_PRIORITY.

        Show
        Naganarasimha Naganarasimha G R added a comment - Well not sure why for all the REST queries are doing audit logs for failures when the app id is not specified. So would be ok for doing this too, IMO is its a invalid input from user end and also not an update activity hence not sure why audit log is required. other than that latest patch looks fine except for the nit that RMAuditLogger.UPDATE_APP_PRIORITY can be used in RMWebServices.updateApplicationPriority method instead of new constant APP_PRIORITY .
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 17s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
        +1 mvninstall 7m 35s trunk passed
        +1 compile 0m 38s trunk passed
        +1 checkstyle 0m 23s trunk passed
        +1 mvnsite 0m 42s trunk passed
        +1 mvneclipse 0m 19s trunk passed
        +1 findbugs 1m 8s trunk passed
        +1 javadoc 0m 24s trunk passed
        +1 mvninstall 0m 36s the patch passed
        +1 compile 0m 32s the patch passed
        +1 javac 0m 32s the patch passed
        +1 checkstyle 0m 20s the patch passed
        +1 mvnsite 0m 42s the patch passed
        +1 mvneclipse 0m 14s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 14s the patch passed
        +1 javadoc 0m 18s the patch passed
        -1 unit 48m 9s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 15s The patch does not generate ASF License warnings.
        64m 27s



        Reason Tests
        Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.TestSchedulingWithAllocationRequestId



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830299/YARN-5660.patch
        JIRA Issue YARN-5660
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 82f2e98bab40 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 14a696f
        Default Java 1.8.0_101
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-YARN-Build/13217/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13217/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13217/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13217/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 mvninstall 7m 35s trunk passed +1 compile 0m 38s trunk passed +1 checkstyle 0m 23s trunk passed +1 mvnsite 0m 42s trunk passed +1 mvneclipse 0m 19s trunk passed +1 findbugs 1m 8s trunk passed +1 javadoc 0m 24s trunk passed +1 mvninstall 0m 36s the patch passed +1 compile 0m 32s the patch passed +1 javac 0m 32s the patch passed +1 checkstyle 0m 20s the patch passed +1 mvnsite 0m 42s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 14s the patch passed +1 javadoc 0m 18s the patch passed -1 unit 48m 9s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 64m 27s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.TestSchedulingWithAllocationRequestId Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830299/YARN-5660.patch JIRA Issue YARN-5660 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 82f2e98bab40 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 14a696f Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/13217/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13217/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13217/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/13217/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        There were more APIs had similar issue. I fixed all of them by introducing new Audit constants. An also updated message reflect corresponding APIs.

        Show
        rohithsharma Rohith Sharma K S added a comment - There were more APIs had similar issue. I fixed all of them by introducing new Audit constants. An also updated message reflect corresponding APIs.
        Hide
        sunilg Sunil G added a comment -

        Thanks Rohith Sharma K S for the patch.
        Generally looks fine.

        Show
        sunilg Sunil G added a comment - Thanks Rohith Sharma K S for the patch. Generally looks fine.
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Thanks Rohith Sharma K S for the latest patch,
        +1 LGTM, will commit it if no further comments ...

        Show
        Naganarasimha Naganarasimha G R added a comment - Thanks Rohith Sharma K S for the latest patch, +1 LGTM, will commit it if no further comments ...
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 26s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
        +1 mvninstall 7m 20s trunk passed
        +1 compile 0m 32s trunk passed
        +1 checkstyle 0m 22s trunk passed
        +1 mvnsite 0m 39s trunk passed
        +1 mvneclipse 0m 17s trunk passed
        +1 findbugs 0m 57s trunk passed
        +1 javadoc 0m 22s trunk passed
        +1 mvninstall 0m 30s the patch passed
        +1 compile 0m 30s the patch passed
        +1 javac 0m 30s the patch passed
        -1 checkstyle 0m 19s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 60 unchanged - 1 fixed = 61 total (was 61)
        +1 mvnsite 0m 36s the patch passed
        +1 mvneclipse 0m 14s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 2s the patch passed
        +1 javadoc 0m 18s the patch passed
        -1 unit 33m 33s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 17s The patch does not generate ASF License warnings.
        48m 57s



        Reason Tests
        Failed junit tests hadoop.yarn.server.resourcemanager.applicationsmanager.TestAMRestart



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830365/YARN-5660.1.patch
        JIRA Issue YARN-5660
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux ade478ea3bcb 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 4815d02
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13219/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/13219/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13219/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13219/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13219/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 26s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 mvninstall 7m 20s trunk passed +1 compile 0m 32s trunk passed +1 checkstyle 0m 22s trunk passed +1 mvnsite 0m 39s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 0m 57s trunk passed +1 javadoc 0m 22s trunk passed +1 mvninstall 0m 30s the patch passed +1 compile 0m 30s the patch passed +1 javac 0m 30s the patch passed -1 checkstyle 0m 19s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 60 unchanged - 1 fixed = 61 total (was 61) +1 mvnsite 0m 36s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 2s the patch passed +1 javadoc 0m 18s the patch passed -1 unit 33m 33s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 48m 57s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.applicationsmanager.TestAMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830365/YARN-5660.1.patch JIRA Issue YARN-5660 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ade478ea3bcb 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 4815d02 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13219/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13219/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13219/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13219/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/13219/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10495 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10495/)
        YARN-5660. Wrong audit constants are used in Get/Put of priority in (naganarasimha_gr: rev ebf528c443f3931c8b0f92ee76615fc203141675)

        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/RMWebServices.java
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/RMAuditLogger.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10495 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10495/ ) YARN-5660 . Wrong audit constants are used in Get/Put of priority in (naganarasimha_gr: rev ebf528c443f3931c8b0f92ee76615fc203141675) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/RMWebServices.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/RMAuditLogger.java
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        thanks for the contribution Rohith Sharma K S and review from Sunil G

        Show
        Naganarasimha Naganarasimha G R added a comment - thanks for the contribution Rohith Sharma K S and review from Sunil G

          People

          • Assignee:
            rohithsharma Rohith Sharma K S
            Reporter:
            rohithsharma Rohith Sharma K S
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development