Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-7054 Yarn Service Phase 2
  3. YARN-8126

Support auto-spawning of admin configured services during bootstrap of RM

Details

    • Sub-task
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • None
    • 3.2.0, 3.1.1
    • None
    • None
    • Reviewed

    Description

      YARN-8048 adds support auto-spawning of admin configured services during bootstrap of rm.

      This JIRA is to follow up some of the comments discussed in YARN-8048.

      Attachments

        1. YARN-8126.002.patch
          22 kB
          Rohith Sharma K S
        2. YARN-8126.001.patch
          20 kB
          Rohith Sharma K S

        Issue Links

          Activity

            Copy pasting gsaha comments from YARN-8048.

            1. SystemServiceManagerImpl.java

                      if (!services.add(service)) {
                        int count = ignoredUserServices.containsKey(userName) ?
                            ignoredUserServices.get(userName) : 0;
                        ignoredUserServices.put(userName, count + 1);
                        LOG.warn(
                            "Ignoring service {} for the user {} as it is already present,"
                                + " filename = {}", service.getName(), userName, filename);
                      }
                      LOG.info("Added service {} for the user {}, filename = {}",
                          service.getName(), userName, filename);
            

            I think the info log will get printed here every time the warn log is also printed inside the if block. Should the info log go in the else block?

            2. Should we rename TestSystemServiceImpl.java to TestSystemServiceManagerImpl.java?

            3. TestSystemServiceImpl

                    Assert.assertTrue(
                        "Service name doesn't exist in expected " + "userService "
                            + serviceNames, serviceNames.contains(next.getName()));
            

            Should we combine these 2 strings into 1 -> "Service name doesn't exist in expected " + "userService "

            4. Do we need to add documentation about "yarn.service.system-service.dir” anywhere?

            5. Under hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/resources/ should we rename the dir “users" to "system-services" to better reflect what these files/tests are for (and change resourcePath in TestSystemServiceImpl.java accordingly)

            6. For an additional test to see if a dir is skipped, do you want to add a directory named “bad" (probably needs a dummy file under it otherwise github will not allow you to commit an empty dir) under the path hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/resources//users/sync/user1/?

            rohithsharma Rohith Sharma K S added a comment - Copy pasting gsaha comments from YARN-8048 . 1. SystemServiceManagerImpl.java if (!services.add(service)) { int count = ignoredUserServices.containsKey(userName) ? ignoredUserServices.get(userName) : 0; ignoredUserServices.put(userName, count + 1); LOG.warn( "Ignoring service {} for the user {} as it is already present," + " filename = {}" , service.getName(), userName, filename); } LOG.info( "Added service {} for the user {}, filename = {}" , service.getName(), userName, filename); I think the info log will get printed here every time the warn log is also printed inside the if block. Should the info log go in the else block? 2. Should we rename TestSystemServiceImpl.java to TestSystemServiceManagerImpl.java? 3. TestSystemServiceImpl Assert.assertTrue( "Service name doesn't exist in expected " + "userService " + serviceNames, serviceNames.contains(next.getName())); Should we combine these 2 strings into 1 -> "Service name doesn't exist in expected " + "userService " 4. Do we need to add documentation about "yarn.service.system-service.dir” anywhere? 5. Under hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/resources/ should we rename the dir “users" to "system-services" to better reflect what these files/tests are for (and change resourcePath in TestSystemServiceImpl.java accordingly) 6. For an additional test to see if a dir is skipped, do you want to add a directory named “bad" (probably needs a dummy file under it otherwise github will not allow you to commit an empty dir) under the path hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/resources//users/sync/user1/?

            Updated the patch fixing follow up review comments.

            rohithsharma Rohith Sharma K S added a comment - Updated the patch fixing follow up review comments.
            genericqa genericqa added a comment -
            +1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 29s Docker mode activated.
                  Prechecks
            +1 @author 0m 0s The patch does not contain any @author tags.
            +1 test4tests 0m 0s The patch appears to include 14 new or modified test files.
                  trunk Compile Tests
            0 mvndep 0m 10s Maven dependency ordering for branch
            +1 mvninstall 25m 33s trunk passed
            +1 compile 8m 53s trunk passed
            +1 checkstyle 1m 19s trunk passed
            +1 mvnsite 1m 32s trunk passed
            +1 shadedclient 13m 7s branch has no errors when building and testing our client artifacts.
            0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
            +1 findbugs 1m 23s trunk passed
            +1 javadoc 1m 9s trunk passed
                  Patch Compile Tests
            0 mvndep 0m 12s Maven dependency ordering for patch
            +1 mvninstall 1m 1s the patch passed
            +1 compile 7m 32s the patch passed
            +1 javac 7m 32s the patch passed
            +1 checkstyle 1m 14s the patch passed
            +1 mvnsite 1m 15s the patch passed
            +1 whitespace 0m 0s The patch has no whitespace issues.
            +1 shadedclient 10m 28s patch has no errors when building and testing our client artifacts.
            0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
            +1 findbugs 1m 31s the patch passed
            +1 javadoc 1m 3s the patch passed
                  Other Tests
            +1 unit 6m 46s hadoop-yarn-services-core in the patch passed.
            +1 unit 0m 38s hadoop-yarn-services-api in the patch passed.
            +1 unit 0m 20s hadoop-yarn-site in the patch passed.
            +1 asflicense 0m 35s The patch does not generate ASF License warnings.
            85m 12s



            Subsystem Report/Notes
            Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8620d2b
            JIRA Issue YARN-8126
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12918044/YARN-8126.001.patch
            Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
            uname Linux 47cba402b202 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9 14:43:09 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
            Build tool maven
            Personality /testptch/patchprocess/precommit/personality/provided.sh
            git revision trunk / 5700556
            maven version: Apache Maven 3.3.9
            Default Java 1.8.0_151
            findbugs v3.1.0-RC1
            Test Results https://builds.apache.org/job/PreCommit-YARN-Build/20269/testReport/
            Max. process+thread count 669 (vs. ulimit of 10000)
            modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
            Console output https://builds.apache.org/job/PreCommit-YARN-Build/20269/console
            Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org

            This message was automatically generated.

            genericqa genericqa added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 29s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 14 new or modified test files.       trunk Compile Tests 0 mvndep 0m 10s Maven dependency ordering for branch +1 mvninstall 25m 33s trunk passed +1 compile 8m 53s trunk passed +1 checkstyle 1m 19s trunk passed +1 mvnsite 1m 32s trunk passed +1 shadedclient 13m 7s branch has no errors when building and testing our client artifacts. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 1m 23s trunk passed +1 javadoc 1m 9s trunk passed       Patch Compile Tests 0 mvndep 0m 12s Maven dependency ordering for patch +1 mvninstall 1m 1s the patch passed +1 compile 7m 32s the patch passed +1 javac 7m 32s the patch passed +1 checkstyle 1m 14s the patch passed +1 mvnsite 1m 15s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedclient 10m 28s patch has no errors when building and testing our client artifacts. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 1m 31s the patch passed +1 javadoc 1m 3s the patch passed       Other Tests +1 unit 6m 46s hadoop-yarn-services-core in the patch passed. +1 unit 0m 38s hadoop-yarn-services-api in the patch passed. +1 unit 0m 20s hadoop-yarn-site in the patch passed. +1 asflicense 0m 35s The patch does not generate ASF License warnings. 85m 12s Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8620d2b JIRA Issue YARN-8126 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12918044/YARN-8126.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle uname Linux 47cba402b202 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9 14:43:09 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/patchprocess/precommit/personality/provided.sh git revision trunk / 5700556 maven version: Apache Maven 3.3.9 Default Java 1.8.0_151 findbugs v3.1.0-RC1 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/20269/testReport/ Max. process+thread count 669 (vs. ulimit of 10000) modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/20269/console Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
            gsaha Gour Saha added a comment -

            rohithsharma the patch looks good. Few minor comments -

            SystemServiceManagerImpl.java

            getbadDirSkipCounter make b in bad uppercase

            Configurations.md

            All service AM specific configs go here. If I understand correctly yarn.service.system-service.dir is a cluster-specific config, right?

            Also, thanks for deleting TestSystemServiceManager.java which had all upgrade specific tests. I think I missed this in my first round review

            gsaha Gour Saha added a comment - rohithsharma the patch looks good. Few minor comments - SystemServiceManagerImpl.java getbadDirSkipCounter make b in bad uppercase Configurations.md All service AM specific configs go here. If I understand correctly yarn.service.system-service.dir is a cluster-specific config, right? Also, thanks for deleting TestSystemServiceManager.java which had all upgrade specific tests. I think I missed this in my first round review

            If I understand correctly yarn.service.system-service.dir is a cluster-specific config, right?

            Yes, How about adding new section Quick start?

            rohithsharma Rohith Sharma K S added a comment - If I understand correctly yarn.service.system-service.dir is a cluster-specific config, right? Yes, How about adding new section Quick start?
            gsaha Gour Saha added a comment -

            I think this deserves to be a new sub-topic "System Services" on the left panel under "Service Discovery" (in the "YARN Service" section). It might seem that there is not much information for it to go to a new page, but there are primarily 2 reasons I am inclining towards it -

            1. An ordinary end-user cannot create or add services to be started as system-services. So it should not be in the existing pages which focuses on what ordinary end-users can do. Hence in this new page, we should specifically call out that this is a cluster admin feature.
            2. This is a pretty handy feature and going forward this page might grow as we add more system-service related features or add helpful system-services to the framework itself and would also need documentation to go with it.

            What do you think?

            gsaha Gour Saha added a comment - I think this deserves to be a new sub-topic "System Services" on the left panel under "Service Discovery" (in the "YARN Service" section). It might seem that there is not much information for it to go to a new page, but there are primarily 2 reasons I am inclining towards it - An ordinary end-user cannot create or add services to be started as system-services. So it should not be in the existing pages which focuses on what ordinary end-users can do. Hence in this new page, we should specifically call out that this is a cluster admin feature. This is a pretty handy feature and going forward this page might grow as we add more system-service related features or add helpful system-services to the framework itself and would also need documentation to go with it. What do you think?

            It make sense to me to add new section though content may not be too much. As you said, it would grow down the line. I will update the patch with above document change.

            rohithsharma Rohith Sharma K S added a comment - It make sense to me to add new section though content may not be too much. As you said, it would grow down the line. I will update the patch with above document change.
            genericqa genericqa added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 38s Docker mode activated.
                  Prechecks
            +1 @author 0m 0s The patch does not contain any @author tags.
            +1 test4tests 0m 0s The patch appears to include 14 new or modified test files.
                  trunk Compile Tests
            0 mvndep 0m 19s Maven dependency ordering for branch
            +1 mvninstall 25m 46s trunk passed
            +1 compile 28m 9s trunk passed
            +1 checkstyle 3m 10s trunk passed
            +1 mvnsite 2m 0s trunk passed
            +1 shadedclient 16m 6s branch has no errors when building and testing our client artifacts.
            0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-project hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
            +1 findbugs 1m 20s trunk passed
            +1 javadoc 1m 33s trunk passed
                  Patch Compile Tests
            0 mvndep 0m 17s Maven dependency ordering for patch
            +1 mvninstall 1m 11s the patch passed
            +1 compile 28m 2s the patch passed
            +1 javac 28m 2s the patch passed
            +1 checkstyle 3m 16s the patch passed
            +1 mvnsite 1m 57s the patch passed
            -1 whitespace 0m 0s The patch has 3 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
            +1 xml 0m 1s The patch has no ill-formed XML file.
            +1 shadedclient 10m 19s patch has no errors when building and testing our client artifacts.
            0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-project hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
            +1 findbugs 1m 40s the patch passed
            +1 javadoc 1m 29s the patch passed
                  Other Tests
            +1 unit 0m 22s hadoop-project in the patch passed.
            +1 unit 6m 57s hadoop-yarn-services-core in the patch passed.
            +1 unit 0m 43s hadoop-yarn-services-api in the patch passed.
            +1 unit 0m 26s hadoop-yarn-site in the patch passed.
            +1 asflicense 0m 45s The patch does not generate ASF License warnings.
            133m 56s



            Subsystem Report/Notes
            Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8620d2b
            JIRA Issue YARN-8126
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12918731/YARN-8126.002.patch
            Optional Tests asflicense mvnsite xml compile javac javadoc mvninstall unit shadedclient findbugs checkstyle
            uname Linux ae540287f82d 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9 14:43:09 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
            Build tool maven
            Personality /testptch/patchprocess/precommit/personality/provided.sh
            git revision trunk / 113af12
            maven version: Apache Maven 3.3.9
            Default Java 1.8.0_151
            findbugs v3.1.0-RC1
            whitespace https://builds.apache.org/job/PreCommit-YARN-Build/20323/artifact/out/whitespace-eol.txt
            Test Results https://builds.apache.org/job/PreCommit-YARN-Build/20323/testReport/
            Max. process+thread count 671 (vs. ulimit of 10000)
            modules C: hadoop-project hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: .
            Console output https://builds.apache.org/job/PreCommit-YARN-Build/20323/console
            Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org

            This message was automatically generated.

            genericqa genericqa added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 38s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 14 new or modified test files.       trunk Compile Tests 0 mvndep 0m 19s Maven dependency ordering for branch +1 mvninstall 25m 46s trunk passed +1 compile 28m 9s trunk passed +1 checkstyle 3m 10s trunk passed +1 mvnsite 2m 0s trunk passed +1 shadedclient 16m 6s branch has no errors when building and testing our client artifacts. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-project hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 1m 20s trunk passed +1 javadoc 1m 33s trunk passed       Patch Compile Tests 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 1m 11s the patch passed +1 compile 28m 2s the patch passed +1 javac 28m 2s the patch passed +1 checkstyle 3m 16s the patch passed +1 mvnsite 1m 57s the patch passed -1 whitespace 0m 0s The patch has 3 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 xml 0m 1s The patch has no ill-formed XML file. +1 shadedclient 10m 19s patch has no errors when building and testing our client artifacts. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-project hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 1m 40s the patch passed +1 javadoc 1m 29s the patch passed       Other Tests +1 unit 0m 22s hadoop-project in the patch passed. +1 unit 6m 57s hadoop-yarn-services-core in the patch passed. +1 unit 0m 43s hadoop-yarn-services-api in the patch passed. +1 unit 0m 26s hadoop-yarn-site in the patch passed. +1 asflicense 0m 45s The patch does not generate ASF License warnings. 133m 56s Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8620d2b JIRA Issue YARN-8126 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12918731/YARN-8126.002.patch Optional Tests asflicense mvnsite xml compile javac javadoc mvninstall unit shadedclient findbugs checkstyle uname Linux ae540287f82d 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9 14:43:09 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/patchprocess/precommit/personality/provided.sh git revision trunk / 113af12 maven version: Apache Maven 3.3.9 Default Java 1.8.0_151 findbugs v3.1.0-RC1 whitespace https://builds.apache.org/job/PreCommit-YARN-Build/20323/artifact/out/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/20323/testReport/ Max. process+thread count 671 (vs. ulimit of 10000) modules C: hadoop-project hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: . Console output https://builds.apache.org/job/PreCommit-YARN-Build/20323/console Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
            gsaha Gour Saha added a comment -

            rohithsharma patch 002 looks great. +1 to commit after you remove the trailing whitespaces from 3 lines in SystemServices.md.

            gsaha Gour Saha added a comment - rohithsharma patch 002 looks great. +1 to commit after you remove the trailing whitespaces from 3 lines in SystemServices.md.

            Thanks gsaha. Reg the white space issue, this could be handled at the time of commit by using command git apply --whitespace=fix -v patch_file

            rohithsharma Rohith Sharma K S added a comment - Thanks gsaha . Reg the white space issue, this could be handled at the time of commit by using command git apply --whitespace=fix -v patch_file

            sunilg Could you please help to commit this patch? And please use above command so that whitespace also will be fixed.

            rohithsharma Rohith Sharma K S added a comment - sunilg Could you please help to commit this patch? And please use above command so that whitespace also will be fixed.
            sunilg Sunil G added a comment -

            +1. Committing shortly.

            sunilg Sunil G added a comment - +1. Committing shortly.
            sunilg Sunil G added a comment -

            Thanks rohithsharma and gsaha. Committed to trunk/3.1

            sunilg Sunil G added a comment - Thanks rohithsharma and gsaha . Committed to trunk/3.1

            People

              rohithsharma Rohith Sharma K S
              rohithsharma Rohith Sharma K S
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: