Uploaded image for project: 'Oozie'
  1. Oozie
  2. OOZIE-2030

Configuration properties from global section is not getting set in Hadoop job conf when using sub-workflow action in Oozie workflow.xml

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.3.0
    • Component/s: action
    • Labels:
      None

      Description

      When submitting Oozie workflow with sub-workflow action and with global section, configuration properties defined in global section is not getting set in launched Hadoop job conf. But when we use Pig or MR action in workflow.xml, configuration properties from global section set properly into Hadoop job conf.

      1. OOZIE-2030-v12.patch
        18 kB
        Jaydeep Vishwakarma
      2. OOZIE-2030-v11.patch
        18 kB
        Jaydeep Vishwakarma
      3. OOZIE-2030-v10.patch
        18 kB
        Jaydeep Vishwakarma
      4. OOZIE-2030-v9.patch
        19 kB
        Jaydeep Vishwakarma
      5. OOZIE-2030-v8.patch
        18 kB
        Jaydeep Vishwakarma
      6. OOZIE-2030-v8.patch
        18 kB
        Jaydeep Vishwakarma
      7. OOZIE-2030-v7.patch
        12 kB
        Jaydeep Vishwakarma
      8. OOZIE-2030-v6.patch
        10 kB
        Jaydeep Vishwakarma
      9. OOZIE-2030-v5.patch
        10 kB
        Jaydeep Vishwakarma
      10. OOZIE-2030-v4.patch
        18 kB
        Jaydeep Vishwakarma
      11. OOZIE-2030-v3.patch
        11 kB
        Jaydeep Vishwakarma
      12. OOZIE-2030-v2.patch
        36 kB
        Shwetha G S
      13. OOZIE-2030.patch
        22 kB
        Shwetha G S

        Issue Links

          Activity

          Hide
          hadoopqa Hadoop QA added a comment -

          Testing JIRA OOZIE-2030

          Cleaning local git workspace

          ----------------------------

          Show
          hadoopqa Hadoop QA added a comment - Testing JIRA OOZIE-2030 Cleaning local git workspace ----------------------------
          Hide
          hadoopqa Hadoop QA added a comment -

          Testing JIRA OOZIE-2030

          Cleaning local git workspace

          ----------------------------

          Show
          hadoopqa Hadoop QA added a comment - Testing JIRA OOZIE-2030 Cleaning local git workspace ----------------------------
          Hide
          shwethags Shwetha G S added a comment -

          Rohini Palaniswamy, Purshotam Shah, I am adding properties from configuration and job-xml as launcher job properties for java actions. Is that ok for your usecases? Else, I can enable this based on config. Please check

          Show
          shwethags Shwetha G S added a comment - Rohini Palaniswamy , Purshotam Shah , I am adding properties from configuration and job-xml as launcher job properties for java actions. Is that ok for your usecases? Else, I can enable this based on config. Please check
          Hide
          hadoopqa Hadoop QA added a comment -

          Testing JIRA OOZIE-2030

          Cleaning local git workspace

          ----------------------------

          +1 PATCH_APPLIES
          +1 CLEAN
          -1 RAW_PATCH_ANALYSIS
          . +1 the patch does not introduce any @author tags
          . +1 the patch does not introduce any tabs
          . +1 the patch does not introduce any trailing spaces
          . -1 the patch contains 1 line(s) longer than 132 characters
          . +1 the patch does adds/modifies 2 testcase(s)
          +1 RAT
          . +1 the patch does not seem to introduce new RAT warnings
          +1 JAVADOC
          . +1 the patch does not seem to introduce new Javadoc warnings
          +1 COMPILE
          . +1 HEAD compiles
          . +1 patch compiles
          . +1 the patch does not seem to introduce new javac warnings
          +1 BACKWARDS_COMPATIBILITY
          . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations
          . +1 the patch does not modify JPA files
          -1 TESTS - patch does not compile, cannot run testcases
          +1 DISTRO
          . +1 distro tarball builds with the patch

          ----------------------------
          -1 Overall result, please check the reported -1(s)

          The full output of the test-patch run is available at

          . https://builds.apache.org/job/oozie-trunk-precommit-build/2120/

          Show
          hadoopqa Hadoop QA added a comment - Testing JIRA OOZIE-2030 Cleaning local git workspace ---------------------------- +1 PATCH_APPLIES +1 CLEAN -1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . -1 the patch contains 1 line(s) longer than 132 characters . +1 the patch does adds/modifies 2 testcase(s) +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 BACKWARDS_COMPATIBILITY . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations . +1 the patch does not modify JPA files -1 TESTS - patch does not compile, cannot run testcases +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- -1 Overall result, please check the reported -1(s) The full output of the test-patch run is available at . https://builds.apache.org/job/oozie-trunk-precommit-build/2120/
          Hide
          shwethags Shwetha G S added a comment -

          actually all the tests ran, but spark tests failed because of OOZIE-2087. No issue with this patch

          Show
          shwethags Shwetha G S added a comment - actually all the tests ran, but spark tests failed because of OOZIE-2087 . No issue with this patch
          Hide
          hadoopqa Hadoop QA added a comment -

          Testing JIRA OOZIE-2030

          Cleaning local git workspace

          ----------------------------

          +1 PATCH_APPLIES
          +1 CLEAN
          -1 RAW_PATCH_ANALYSIS
          . +1 the patch does not introduce any @author tags
          . +1 the patch does not introduce any tabs
          . +1 the patch does not introduce any trailing spaces
          . -1 the patch contains 1 line(s) longer than 132 characters
          . +1 the patch does adds/modifies 2 testcase(s)
          +1 RAT
          . +1 the patch does not seem to introduce new RAT warnings
          +1 JAVADOC
          . +1 the patch does not seem to introduce new Javadoc warnings
          +1 COMPILE
          . +1 HEAD compiles
          . +1 patch compiles
          . +1 the patch does not seem to introduce new javac warnings
          +1 BACKWARDS_COMPATIBILITY
          . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations
          . +1 the patch does not modify JPA files
          -1 TESTS
          . Tests run: 1558
          . Tests failed: 1
          . Tests errors: 0

          . The patch failed the following testcases:

          . testMessage_withMixedStatus(org.apache.oozie.command.coord.TestAbandonedCoordChecker)

          +1 DISTRO
          . +1 distro tarball builds with the patch

          ----------------------------
          -1 Overall result, please check the reported -1(s)

          The full output of the test-patch run is available at

          . https://builds.apache.org/job/oozie-trunk-precommit-build/2129/

          Show
          hadoopqa Hadoop QA added a comment - Testing JIRA OOZIE-2030 Cleaning local git workspace ---------------------------- +1 PATCH_APPLIES +1 CLEAN -1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . -1 the patch contains 1 line(s) longer than 132 characters . +1 the patch does adds/modifies 2 testcase(s) +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 BACKWARDS_COMPATIBILITY . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations . +1 the patch does not modify JPA files -1 TESTS . Tests run: 1558 . Tests failed: 1 . Tests errors: 0 . The patch failed the following testcases: . testMessage_withMixedStatus(org.apache.oozie.command.coord.TestAbandonedCoordChecker) +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- -1 Overall result, please check the reported -1(s) The full output of the test-patch run is available at . https://builds.apache.org/job/oozie-trunk-precommit-build/2129/
          Hide
          rohini Rohini Palaniswamy added a comment -

          I am adding properties from configuration and job-xml as launcher job properties for java actions

          The standard has been to prefix launcher properties with oozie.launcher. Are we going against that?

          Show
          rohini Rohini Palaniswamy added a comment - I am adding properties from configuration and job-xml as launcher job properties for java actions The standard has been to prefix launcher properties with oozie.launcher. Are we going against that?
          Hide
          shwethags Shwetha G S added a comment -

          Rohini Palaniswamy,
          oozie.launcher.* is added as well.

          Currently, the properties from conf/job-xml are available as a file which can be loaded in java launcher. But they are not available as default conf. Having them as default conf is useful and simpler if java action is used as a driver(which is the case most of the times). This patch adds the properties from conf/job-xm in the launcher job conf and oozie.launcher.* overrides.

          This is not backward compatible. If this doesn't work for some, I can add this based on config. Let me know

          Show
          shwethags Shwetha G S added a comment - Rohini Palaniswamy , oozie.launcher.* is added as well. Currently, the properties from conf/job-xml are available as a file which can be loaded in java launcher. But they are not available as default conf. Having them as default conf is useful and simpler if java action is used as a driver(which is the case most of the times). This patch adds the properties from conf/job-xm in the launcher job conf and oozie.launcher.* overrides. This is not backward compatible. If this doesn't work for some, I can add this based on config. Let me know
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Hi Rohini Palaniswamy and Shwetha G S I have moved parse "job-xml" along with global conf copy on ActionStartXCommand. But some test cases which are parsing "job-xml" through JavaActionExecutor are failing what to do with those test cases. One of test case is testSetupMethods() }} in {{TestJavaActionExecutor.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Hi Rohini Palaniswamy and Shwetha G S I have moved parse "job-xml" along with global conf copy on ActionStartXCommand . But some test cases which are parsing "job-xml" through JavaActionExecutor are failing what to do with those test cases. One of test case is testSetupMethods() }} in {{TestJavaActionExecutor .
          Hide
          rohini Rohini Palaniswamy added a comment -

          Jaydeep Vishwakarma,
          I had comments in the review board on the approach taken by Shwetha in her patch. The patch needs to be reworked a lot. Please refer to my comments there. Parsing global-xml should not be done in ActionExecutor classes. It is the wrong place to do and is also inefficient. It should be done when the sub-workflow action is constructed and stored similar to other workflow actions. That will also make the config to be shown in the Action Configuration section. Will avoid your testcase problems as well.

          Show
          rohini Rohini Palaniswamy added a comment - Jaydeep Vishwakarma , I had comments in the review board on the approach taken by Shwetha in her patch. The patch needs to be reworked a lot. Please refer to my comments there. Parsing global-xml should not be done in ActionExecutor classes. It is the wrong place to do and is also inefficient. It should be done when the sub-workflow action is constructed and stored similar to other workflow actions. That will also make the config to be shown in the Action Configuration section. Will avoid your testcase problems as well.
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Yes Rohini Palaniswamy, I have check the comment in review board, Had discussion with Shwetha G S. I am taking global conf from SubWorkflowActionExecutor and parsing job-xml in ActionStartXCommand. Now to problem is there are few Test cases where we are parsing job-xml in JavaActionExecutor , What to do with them. You can Refer TestJavaActionExecutor for same.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Yes Rohini Palaniswamy , I have check the comment in review board, Had discussion with Shwetha G S . I am taking global conf from SubWorkflowActionExecutor and parsing job-xml in ActionStartXCommand . Now to problem is there are few Test cases where we are parsing job-xml in JavaActionExecutor , What to do with them. You can Refer TestJavaActionExecutor for same.
          Hide
          rohini Rohini Palaniswamy added a comment -

          ActionStartXCommand again would not be the place to do it. Parsing of globals is done in LiteWorkflowAppParser.handleGlobal and anything new should be handled there. Earlier I had commented about make it aware of parent workflow global section as well. Actually even that is not required and makes it complicated. The simplest and cleanest fix with very less changes to code would be to inject a global section into the sub workflow before submitting it. This will then take the normal code path for global section.

          SubWorkflowActionExecutor.java

          if (eConf.getChild(("propagate-configuration"), ns) != null) {
                              XConfiguration.copy(parentConf, subWorkflowConf); // This merges configuration section
                              // Add a method which injects or merges global section here. If no existing global section, copy <global> element as is from parent. If existing, merge the global section from parent workflow. The merging should give preference to the sub-workflow <namenode>, <jobtracker> and <configuration> in <global> and not override it. Only new configuration should be added to existing <configuration>. Any <job-xml> in parent workflow <global> should be added before the ones in sub-workflow when merging.
                          }
          

          Basically if parent workflow had

          <global>
             <job-tracker>${job-tracker}</job-tracker>
             <name-node>${namd-node}</name-node>
             <job-xml>job1.xml</job-xml>
             <configuration>
                  <property>
                      <name>mapred.job.queue.name</name>
                      <value>${queueName}</value>
                  </property>
                  <property>
                      <name>dfs.umaskmode</name>
                      <value>077</value>
                  </property>
              </configuration>
          </global>
          

          and sub-workflow had

          <global>
             <job-tracker>${job-tracker1}</job-tracker>
             <job-xml>job2.xml</job-xml>
             <configuration>
                  <property>
                      <name>dfs.umaskmode</name>
                      <value>022</value>
                  </property>
              </configuration>
          </global>
          

          After merging sub-workflow should have

          <global>
             <job-tracker>${job-tracker1}</job-tracker>
             <name-node>${namd-node}</name-node>
             <job-xml>job1.xml</job-xml>
             <job-xml>job2.xml</job-xml>
             <configuration>
                  <property>
                      <name>mapred.job.queue.name</name>
                      <value>${queueName}</value>
                  </property>
                  <property>
                      <name>dfs.umaskmode</name>
                      <value>022</value>
                  </property>
              </configuration>
          </global>
          
          Show
          rohini Rohini Palaniswamy added a comment - ActionStartXCommand again would not be the place to do it. Parsing of globals is done in LiteWorkflowAppParser.handleGlobal and anything new should be handled there. Earlier I had commented about make it aware of parent workflow global section as well. Actually even that is not required and makes it complicated. The simplest and cleanest fix with very less changes to code would be to inject a global section into the sub workflow before submitting it. This will then take the normal code path for global section. SubWorkflowActionExecutor.java if (eConf.getChild(( "propagate-configuration" ), ns) != null ) { XConfiguration.copy(parentConf, subWorkflowConf); // This merges configuration section // Add a method which injects or merges global section here. If no existing global section, copy <global> element as is from parent. If existing, merge the global section from parent workflow. The merging should give preference to the sub-workflow <namenode>, <jobtracker> and <configuration> in <global> and not override it. Only new configuration should be added to existing <configuration>. Any <job-xml> in parent workflow <global> should be added before the ones in sub-workflow when merging. } Basically if parent workflow had <global> <job-tracker>${job-tracker}</job-tracker> <name-node>${namd-node}</name-node> <job-xml>job1.xml</job-xml> <configuration> <property> <name>mapred.job.queue.name</name> <value>${queueName}</value> </property> <property> <name>dfs.umaskmode</name> <value>077</value> </property> </configuration> </global> and sub-workflow had <global> <job-tracker>${job-tracker1}</job-tracker> <job-xml>job2.xml</job-xml> <configuration> <property> <name>dfs.umaskmode</name> <value>022</value> </property> </configuration> </global> After merging sub-workflow should have <global> <job-tracker>${job-tracker1}</job-tracker> <name-node>${namd-node}</name-node> <job-xml>job1.xml</job-xml> <job-xml>job2.xml</job-xml> <configuration> <property> <name>mapred.job.queue.name</name> <value>${queueName}</value> </property> <property> <name>dfs.umaskmode</name> <value>022</value> </property> </configuration> </global>
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Rohini Palaniswamy, I have done the changes as per you suggested , Please have a look.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Rohini Palaniswamy , I have done the changes as per you suggested , Please have a look.
          Hide
          hadoopqa Hadoop QA added a comment -

          Testing JIRA OOZIE-2030

          Cleaning local git workspace

          ----------------------------

          +1 PATCH_APPLIES
          +1 CLEAN
          -1 RAW_PATCH_ANALYSIS
          . +1 the patch does not introduce any @author tags
          . +1 the patch does not introduce any tabs
          . +1 the patch does not introduce any trailing spaces
          . -1 the patch contains 2 line(s) longer than 132 characters
          . +1 the patch does adds/modifies 1 testcase(s)
          +1 RAT
          . +1 the patch does not seem to introduce new RAT warnings
          +1 JAVADOC
          . +1 the patch does not seem to introduce new Javadoc warnings
          +1 COMPILE
          . +1 HEAD compiles
          . +1 patch compiles
          . +1 the patch does not seem to introduce new javac warnings
          +1 BACKWARDS_COMPATIBILITY
          . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations
          . +1 the patch does not modify JPA files
          -1 TESTS
          . Tests run: 1654
          . Tests failed: 3
          . Tests errors: 3

          . The patch failed the following testcases:

          . testNone(org.apache.oozie.command.coord.TestCoordActionInputCheckXCommandNonUTC)
          . testPauseBundleAndCoordinator(org.apache.oozie.service.TestPauseTransitService)
          . testCoord_throwException(org.apache.oozie.command.coord.TestCoordChangeXCommand)

          +1 DISTRO
          . +1 distro tarball builds with the patch

          ----------------------------
          -1 Overall result, please check the reported -1(s)

          The full output of the test-patch run is available at

          . https://builds.apache.org/job/oozie-trunk-precommit-build/2377/

          Show
          hadoopqa Hadoop QA added a comment - Testing JIRA OOZIE-2030 Cleaning local git workspace ---------------------------- +1 PATCH_APPLIES +1 CLEAN -1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . -1 the patch contains 2 line(s) longer than 132 characters . +1 the patch does adds/modifies 1 testcase(s) +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 BACKWARDS_COMPATIBILITY . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations . +1 the patch does not modify JPA files -1 TESTS . Tests run: 1654 . Tests failed: 3 . Tests errors: 3 . The patch failed the following testcases: . testNone(org.apache.oozie.command.coord.TestCoordActionInputCheckXCommandNonUTC) . testPauseBundleAndCoordinator(org.apache.oozie.service.TestPauseTransitService) . testCoord_throwException(org.apache.oozie.command.coord.TestCoordChangeXCommand) +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- -1 Overall result, please check the reported -1(s) The full output of the test-patch run is available at . https://builds.apache.org/job/oozie-trunk-precommit-build/2377/
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Test failures are unrelated with this patch.
          2 long ling cannot avoid from the patch.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Test failures are unrelated with this patch. 2 long ling cannot avoid from the patch.
          Hide
          rohini Rohini Palaniswamy added a comment -

          You do not have to make changes to LiteWorkflowAppParser.java. My last comment was to modify the <global> xml element of sub-workflow in SubWorkflowActionExecutor.java itself to include (merge) the parent workflows global configuration if propagate-configuration is true. That would be similar to how the <configuration> xml element of sub-workflow is modified to include the parentw workflows configuration properties as well. Your patch prints the <global> section into a configuration and parses that later. This is not needed and will not work correctly with multiple levels of sub-workflows. Please modify the <global> xml in SubWorkflowActionExecutor.java itself. Also sub-workflow's global conf (both job.xml and configuration property) should have precedence over the parent's. From your current patch looks like the parent will have precedence. Ensure you have a test case for that.

          Show
          rohini Rohini Palaniswamy added a comment - You do not have to make changes to LiteWorkflowAppParser.java. My last comment was to modify the <global> xml element of sub-workflow in SubWorkflowActionExecutor.java itself to include (merge) the parent workflows global configuration if propagate-configuration is true. That would be similar to how the <configuration> xml element of sub-workflow is modified to include the parentw workflows configuration properties as well. Your patch prints the <global> section into a configuration and parses that later. This is not needed and will not work correctly with multiple levels of sub-workflows. Please modify the <global> xml in SubWorkflowActionExecutor.java itself. Also sub-workflow's global conf (both job.xml and configuration property) should have precedence over the parent's. From your current patch looks like the parent will have precedence. Ensure you have a test case for that.
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          I tried to follow same approach, But global element of sub-workflow and parent workflow is not coming in SubWorkflowActionExecutor.java.
          I have done some changes LiteWorkflowAppParser.java this will ensure for multi level pass of global element and will give the priority to sub-workflow properties. To verify the priority of sub-workflow properties i have modified the test case as well.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - I tried to follow same approach, But global element of sub-workflow and parent workflow is not coming in SubWorkflowActionExecutor.java. I have done some changes LiteWorkflowAppParser.java this will ensure for multi level pass of global element and will give the priority to sub-workflow properties. To verify the priority of sub-workflow properties i have modified the test case as well.
          Hide
          sriksun Srikanth Sundarrajan added a comment -

          Jaydeep Vishwakarma, Context arg in Subworkflow should give you a handle to the parent workflow via context::getWorkflow(). Shouldn't you be accessing the global section of the parent workflow here instead of LiteWorkflowAppParser::parse(). Mutating a single property
          via jobConf.get(SubWorkflowActionExecutor.SUBWF_JOBCONF) isn't likely to work when subflow depth is more than one, as the same will be overwritten.

                          XConfiguration subWorkflowConf = new XConfiguration();
                          Configuration parentConf = new XConfiguration(new StringReader(context.getWorkflow().getConf()));
                          if (eConf.getChild(("propagate-configuration"), ns) != null) {
                              XConfiguration.copy(parentConf, subWorkflowConf);
                          }
          

          Also the test case isn't really testing for precedence of global overlays. The current test has mutually exclusive conf in the parent & subflow global section and with this test case it is not possible to assert that the precedence is being honored correctly.

          Show
          sriksun Srikanth Sundarrajan added a comment - Jaydeep Vishwakarma , Context arg in Subworkflow should give you a handle to the parent workflow via context::getWorkflow() . Shouldn't you be accessing the global section of the parent workflow here instead of LiteWorkflowAppParser::parse() . Mutating a single property via jobConf.get(SubWorkflowActionExecutor.SUBWF_JOBCONF) isn't likely to work when subflow depth is more than one, as the same will be overwritten. XConfiguration subWorkflowConf = new XConfiguration(); Configuration parentConf = new XConfiguration( new StringReader(context.getWorkflow().getConf())); if (eConf.getChild(( "propagate-configuration" ), ns) != null ) { XConfiguration.copy(parentConf, subWorkflowConf); } Also the test case isn't really testing for precedence of global overlays. The current test has mutually exclusive conf in the parent & subflow global section and with this test case it is not possible to assert that the precedence is being honored correctly.
          Hide
          sriksun Srikanth Sundarrajan added a comment -

          (minor nit) There are few unused imports in the patch.

          Shwetha G S, Can you please close the previous review request. This should allow Jaydeep Vishwakarma to create a new review request and upload this revisions.

          Show
          sriksun Srikanth Sundarrajan added a comment - (minor nit) There are few unused imports in the patch. Shwetha G S , Can you please close the previous review request. This should allow Jaydeep Vishwakarma to create a new review request and upload this revisions.
          Hide
          shwethags Shwetha G S added a comment -

          Srikanth Sundarrajan, Closed the old review request. But new review request can be created even otherwise, no?

          Jaydeep Vishwakarma, can you add the patch to review board?

          Show
          shwethags Shwetha G S added a comment - Srikanth Sundarrajan , Closed the old review request. But new review request can be created even otherwise, no? Jaydeep Vishwakarma , can you add the patch to review board?
          Hide
          sriksun Srikanth Sundarrajan added a comment -

          Yes Shwetha G S, didn't want two open review request pending in review board at the same time, hence the request. Thanks

          Show
          sriksun Srikanth Sundarrajan added a comment - Yes Shwetha G S , didn't want two open review request pending in review board at the same time, hence the request. Thanks
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Srikanth Sundarrajan, I have added the same patch in review board.
          Comments on your review.
          context.workflow does not have global section. Handle global already extracting all the global section conf from there. Global conf is merge with other conf. I will not be able to figure out which conf values belongs to global.
          jobConf.get(SubWorkflowActionExecutor.SUBWF_JOBCONF) will be able to go N number of levels without any issue.
          {fetchGlobalConf(ns, parentGlobalElement, global);}} and elem.addContent((Element)global.clone()); ensuring it.

          I am asserting three values in test case, where foo2 belongs to both both workflow global conf. If you check the test case foo2 have subworkflow's value instead of workflow.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Srikanth Sundarrajan , I have added the same patch in review board. Comments on your review. context.workflow does not have global section. Handle global already extracting all the global section conf from there. Global conf is merge with other conf. I will not be able to figure out which conf values belongs to global. jobConf.get(SubWorkflowActionExecutor.SUBWF_JOBCONF) will be able to go N number of levels without any issue. {fetchGlobalConf(ns, parentGlobalElement, global);}} and elem.addContent((Element)global.clone()); ensuring it. I am asserting three values in test case, where foo2 belongs to both both workflow global conf. If you check the test case foo2 have subworkflow's value instead of workflow.
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Srikanth Sundarrajan, I have added four imports in this patch. All of them are being used.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Srikanth Sundarrajan , I have added four imports in this patch. All of them are being used.
          Hide
          sriksun Srikanth Sundarrajan added a comment -

          My bad, I must have confused it with the other patch I was reviewing.

          Show
          sriksun Srikanth Sundarrajan added a comment - My bad, I must have confused it with the other patch I was reviewing.
          Hide
          sriksun Srikanth Sundarrajan added a comment -

          My bad, I must have confused it with the other patch I was reviewing.

          Show
          sriksun Srikanth Sundarrajan added a comment - My bad, I must have confused it with the other patch I was reviewing.
          Hide
          sriksun Srikanth Sundarrajan added a comment -

          We need to have a uniform mechanism to parse global section and make them available to the action executors and then it should be upto the executors on how to use them as they deem fit.

          Show
          sriksun Srikanth Sundarrajan added a comment - We need to have a uniform mechanism to parse global section and make them available to the action executors and then it should be upto the executors on how to use them as they deem fit.
          Hide
          shwethags Shwetha G S added a comment -

          Rohini Palaniswamy,
          SubWorkflowActionExecutor launches new workflow with app path = suborkflow app path. Sub workflow will read the global conf from this subworkflow app path. Merging the global sections requires modifying the workflow.xml from this subworkflow app path which we don't want to touch, right? So, only other way is to set the global conf as a conf while launching subworkflow and workflow parser of sub workflow will do the merge. Makes sense? So, this requires changes in LiteWorkflowAppParser

          Show
          shwethags Shwetha G S added a comment - Rohini Palaniswamy , SubWorkflowActionExecutor launches new workflow with app path = suborkflow app path. Sub workflow will read the global conf from this subworkflow app path. Merging the global sections requires modifying the workflow.xml from this subworkflow app path which we don't want to touch, right? So, only other way is to set the global conf as a conf while launching subworkflow and workflow parser of sub workflow will do the merge. Makes sense? So, this requires changes in LiteWorkflowAppParser
          Hide
          sriksun Srikanth Sundarrajan added a comment -

          There is perhaps a simpler way to tackle this issue. If we modify the LiteWorkflowAppParser to serialize persist the contents of global in conf and have handleGlobal() also consult conf for the handling section, this will ensure that global is propagated correctly with no further changes to any other section of the code, honoring the right overlay priorities. While the code in LWAP wouldn't be specific to Subworkflows either. Luckily the conf itself is propagates into subflows on user's request. - Shwetha G S, Rohini Palaniswamy, makes sense ?

          Show
          sriksun Srikanth Sundarrajan added a comment - There is perhaps a simpler way to tackle this issue. If we modify the LiteWorkflowAppParser to serialize persist the contents of global in conf and have handleGlobal() also consult conf for the handling section, this will ensure that global is propagated correctly with no further changes to any other section of the code, honoring the right overlay priorities. While the code in LWAP wouldn't be specific to Subworkflows either. Luckily the conf itself is propagates into subflows on user's request. - Shwetha G S , Rohini Palaniswamy , makes sense ?
          Hide
          sriksun Srikanth Sundarrajan added a comment -

          Didn't realize that Shwetha G S has already proposed the same approach. Sorry

          Show
          sriksun Srikanth Sundarrajan added a comment - Didn't realize that Shwetha G S has already proposed the same approach. Sorry
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Yes, Srikanth Sundarrajan solution looks more clean.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Yes, Srikanth Sundarrajan solution looks more clean.
          Hide
          rohini Rohini Palaniswamy added a comment -

          Agree. If we are serializing whole global section into conf, need to ensure that the serialized conf does not end up in multiple places - each action conf, job.xml on hdfs, etc which will bloat size a lot and that inefficiency will end up being a problem at scale. So Jaydeep Vishwakarma, in the implementation take care to only process that serialized config in LiteWorkflowAppParser and throw it away before the actionconf is constructed and stored in database. Robert Kanter has done some cleanup of the global section in OOZIE-2187. Would be good to base your patch on top of that.

          Also please base64 encode the setting value, in case it causes some issues with the xml format.

          Show
          rohini Rohini Palaniswamy added a comment - Agree. If we are serializing whole global section into conf, need to ensure that the serialized conf does not end up in multiple places - each action conf, job.xml on hdfs, etc which will bloat size a lot and that inefficiency will end up being a problem at scale. So Jaydeep Vishwakarma , in the implementation take care to only process that serialized config in LiteWorkflowAppParser and throw it away before the actionconf is constructed and stored in database. Robert Kanter has done some cleanup of the global section in OOZIE-2187 . Would be good to base your patch on top of that. Also please base64 encode the setting value, in case it causes some issues with the xml format.
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Added the patch, also uploaded in review board. keeping global element in xml format only, As it helps during debugging the issue.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Added the patch, also uploaded in review board. keeping global element in xml format only, As it helps during debugging the issue.
          Hide
          sriksun Srikanth Sundarrajan added a comment -

          +1

          Show
          sriksun Srikanth Sundarrajan added a comment - +1
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Changes done as per review. Thanks for Review Srikanth Sundarrajan and Shwetha G S

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Changes done as per review. Thanks for Review Srikanth Sundarrajan and Shwetha G S
          Hide
          shwethags Shwetha G S added a comment -

          +1, will commit post jenkins build

          Show
          shwethags Shwetha G S added a comment - +1, will commit post jenkins build
          Show
          shwethags Shwetha G S added a comment - Triggered build: https://builds.apache.org/job/oozie-trunk-precommit-build/2432/
          Hide
          shwethags Shwetha G S added a comment -

          Sorry, missed Rohini's comments:
          1. in the implementation take care to only process that serialized config in LiteWorkflowAppParser and throw it away before the actionconf is constructed and stored in database
          2. Also please base64 encode the setting value, in case it causes some issues with the xml format.

          Jaydeep Vishwakarma, please address them

          Show
          shwethags Shwetha G S added a comment - Sorry, missed Rohini's comments: 1. in the implementation take care to only process that serialized config in LiteWorkflowAppParser and throw it away before the actionconf is constructed and stored in database 2. Also please base64 encode the setting value, in case it causes some issues with the xml format. Jaydeep Vishwakarma , please address them
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Shwetha G S,

          1. in the implementation take care to only process that serialized config in LiteWorkflowAppParser and throw it away before the actionconf is constructed and stored in database.
          I will update the patch.
          2. Also please base64 encode the setting value, in case it causes some issues with the xml format.
          I am keeping it in the text format to improve the debugging.

          Let me know if you have any other concern?

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Shwetha G S , 1. in the implementation take care to only process that serialized config in LiteWorkflowAppParser and throw it away before the actionconf is constructed and stored in database. I will update the patch. 2. Also please base64 encode the setting value, in case it causes some issues with the xml format. I am keeping it in the text format to improve the debugging. Let me know if you have any other concern?
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Rohini Palaniswamy, Shwetha G S,
          What is the patch doing
          1. It is adding oozie.globalconf in workflow if global conf is present
          2. It is adding oozie.globalconf in subworkflow if any of the ancestor workflow have global conf.

          What is being additionally added into Oozie State
          1. It will keep global conf in text format.
          2. Workflow and subworkflow will have additional payload for global conf in DB.

          Why I am inclined to retain the patch as it is
          1. Keep the conf in text format instead of Base64 will improve the debuggability of code.
          2. Removing global conf from workflow after using it will make it hacky. It will make difficult for developer to understand that there was some property which passed to the workflow action.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Rohini Palaniswamy , Shwetha G S , What is the patch doing 1. It is adding oozie.globalconf in workflow if global conf is present 2. It is adding oozie.globalconf in subworkflow if any of the ancestor workflow have global conf. What is being additionally added into Oozie State 1. It will keep global conf in text format. 2. Workflow and subworkflow will have additional payload for global conf in DB. Why I am inclined to retain the patch as it is 1. Keep the conf in text format instead of Base64 will improve the debuggability of code. 2. Removing global conf from workflow after using it will make it hacky. It will make difficult for developer to understand that there was some property which passed to the workflow action.
          Hide
          shwethags Shwetha G S added a comment -

          This data is compressed in DB. So, how will storing in text format help with debugging?

          Show
          shwethags Shwetha G S added a comment - This data is compressed in DB. So, how will storing in text format help with debugging?
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          The data is compressed only when you store in DB , But when it is loaded in bean it is in text format.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - The data is compressed only when you store in DB , But when it is loaded in bean it is in text format.
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Here is how an oozie user can see the config value for workflow.
          oozie job -configcontent <id>

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Here is how an oozie user can see the config value for workflow. oozie job -configcontent <id>
          Hide
          sriksun Srikanth Sundarrajan added a comment -

          From what I understood there were 2 concerns with the patch

          1> Not removing the global conf from workflow conf before persistence
          2> Not encoding global conf and using xml as is

          Given the global section is being added only to the workflow conf (in compressed state), am assuming there isn't much storage overhead and retaining the conf in xml format, might not be all that bad as long as there is no direct string manipulation of xmls.

          Any further work needed on the patch ?

          Show
          sriksun Srikanth Sundarrajan added a comment - From what I understood there were 2 concerns with the patch 1> Not removing the global conf from workflow conf before persistence 2> Not encoding global conf and using xml as is Given the global section is being added only to the workflow conf (in compressed state), am assuming there isn't much storage overhead and retaining the conf in xml format, might not be all that bad as long as there is no direct string manipulation of xmls. Any further work needed on the patch ?
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Some improvement on patch, oozie.globalconf will not be stored in conf if workflow does not have subworkflow action.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Some improvement on patch, oozie.globalconf will not be stored in conf if workflow does not have subworkflow action.
          Hide
          rohini Rohini Palaniswamy added a comment -

          Thanks. Will review today.

          Show
          rohini Rohini Palaniswamy added a comment - Thanks. Will review today.
          Hide
          hadoopqa Hadoop QA added a comment -

          Testing JIRA OOZIE-2030

          Cleaning local git workspace

          ----------------------------

          +1 PATCH_APPLIES
          +1 CLEAN
          +1 RAW_PATCH_ANALYSIS
          . +1 the patch does not introduce any @author tags
          . +1 the patch does not introduce any tabs
          . +1 the patch does not introduce any trailing spaces
          . +1 the patch does not introduce any line longer than 132
          . +1 the patch does adds/modifies 1 testcase(s)
          +1 RAT
          . +1 the patch does not seem to introduce new RAT warnings
          +1 JAVADOC
          . +1 the patch does not seem to introduce new Javadoc warnings
          +1 COMPILE
          . +1 HEAD compiles
          . +1 patch compiles
          . +1 the patch does not seem to introduce new javac warnings
          +1 BACKWARDS_COMPATIBILITY
          . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations
          . +1 the patch does not modify JPA files
          -1 TESTS
          . Tests run: 1674
          . Tests failed: 1
          . Tests errors: 0

          . The patch failed the following testcases:

          . testStreamingWithMultipleOozieServers_coordActionList(org.apache.oozie.service.TestZKXLogStreamingService)

          +1 DISTRO
          . +1 distro tarball builds with the patch

          ----------------------------
          -1 Overall result, please check the reported -1(s)

          The full output of the test-patch run is available at

          . https://builds.apache.org/job/oozie-trunk-precommit-build/2442/

          Show
          hadoopqa Hadoop QA added a comment - Testing JIRA OOZIE-2030 Cleaning local git workspace ---------------------------- +1 PATCH_APPLIES +1 CLEAN +1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . +1 the patch does not introduce any line longer than 132 . +1 the patch does adds/modifies 1 testcase(s) +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 BACKWARDS_COMPATIBILITY . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations . +1 the patch does not modify JPA files -1 TESTS . Tests run: 1674 . Tests failed: 1 . Tests errors: 0 . The patch failed the following testcases: . testStreamingWithMultipleOozieServers_coordActionList(org.apache.oozie.service.TestZKXLogStreamingService) +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- -1 Overall result, please check the reported -1(s) The full output of the test-patch run is available at . https://builds.apache.org/job/oozie-trunk-precommit-build/2442/
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Changes done as review comments.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Changes done as review comments.
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Rohini Palaniswamy , Can you have a look, All open issues have been addressed long time back , This patch is ready to merge.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Rohini Palaniswamy , Can you have a look, All open issues have been addressed long time back , This patch is ready to merge.
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Changes done as per review.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Changes done as per review.
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Thanks Rohini for review, I have done changes as per your feedback

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Thanks Rohini for review, I have done changes as per your feedback
          Hide
          hadoopqa Hadoop QA added a comment -

          Testing JIRA OOZIE-2030

          Cleaning local git workspace

          ----------------------------

          +1 PATCH_APPLIES
          +1 CLEAN
          +1 RAW_PATCH_ANALYSIS
          . +1 the patch does not introduce any @author tags
          . +1 the patch does not introduce any tabs
          . +1 the patch does not introduce any trailing spaces
          . +1 the patch does not introduce any line longer than 132
          . +1 the patch does adds/modifies 1 testcase(s)
          +1 RAT
          . +1 the patch does not seem to introduce new RAT warnings
          +1 JAVADOC
          . +1 the patch does not seem to introduce new Javadoc warnings
          +1 COMPILE
          . +1 HEAD compiles
          . +1 patch compiles
          . +1 the patch does not seem to introduce new javac warnings
          +1 BACKWARDS_COMPATIBILITY
          . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations
          . +1 the patch does not modify JPA files
          -1 TESTS
          . Tests run: 1703
          . Tests failed: 4
          . Tests errors: 4

          . The patch failed the following testcases:

          . testForNoDuplicates(org.apache.oozie.event.TestEventGeneration)
          . testSamplers(org.apache.oozie.util.TestMetricsInstrumentation)
          . testTimeoutWithException(org.apache.oozie.command.coord.TestCoordActionInputCheckXCommandNonUTC)
          . testUpdateSLA(org.apache.oozie.sla.TestSLAService)

          +1 DISTRO
          . +1 distro tarball builds with the patch

          ----------------------------
          -1 Overall result, please check the reported -1(s)

          The full output of the test-patch run is available at

          . https://builds.apache.org/job/oozie-trunk-precommit-build/2607/

          Show
          hadoopqa Hadoop QA added a comment - Testing JIRA OOZIE-2030 Cleaning local git workspace ---------------------------- +1 PATCH_APPLIES +1 CLEAN +1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . +1 the patch does not introduce any line longer than 132 . +1 the patch does adds/modifies 1 testcase(s) +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 BACKWARDS_COMPATIBILITY . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations . +1 the patch does not modify JPA files -1 TESTS . Tests run: 1703 . Tests failed: 4 . Tests errors: 4 . The patch failed the following testcases: . testForNoDuplicates(org.apache.oozie.event.TestEventGeneration) . testSamplers(org.apache.oozie.util.TestMetricsInstrumentation) . testTimeoutWithException(org.apache.oozie.command.coord.TestCoordActionInputCheckXCommandNonUTC) . testUpdateSLA(org.apache.oozie.sla.TestSLAService) +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- -1 Overall result, please check the reported -1(s) The full output of the test-patch run is available at . https://builds.apache.org/job/oozie-trunk-precommit-build/2607/
          Hide
          hadoopqa Hadoop QA added a comment -

          Testing JIRA OOZIE-2030

          Cleaning local git workspace

          ----------------------------

          +1 PATCH_APPLIES
          +1 CLEAN
          +1 RAW_PATCH_ANALYSIS
          . +1 the patch does not introduce any @author tags
          . +1 the patch does not introduce any tabs
          . +1 the patch does not introduce any trailing spaces
          . +1 the patch does not introduce any line longer than 132
          . +1 the patch does adds/modifies 1 testcase(s)
          +1 RAT
          . +1 the patch does not seem to introduce new RAT warnings
          +1 JAVADOC
          . +1 the patch does not seem to introduce new Javadoc warnings
          +1 COMPILE
          . +1 HEAD compiles
          . +1 patch compiles
          . +1 the patch does not seem to introduce new javac warnings
          +1 BACKWARDS_COMPATIBILITY
          . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations
          . +1 the patch does not modify JPA files
          -1 TESTS
          . Tests run: 1703
          . Tests failed: 8
          . Tests errors: 6

          . The patch failed the following testcases:

          . testForNoDuplicates(org.apache.oozie.event.TestEventGeneration)
          . testSamplers(org.apache.oozie.util.TestMetricsInstrumentation)
          . testUpdateSLA(org.apache.oozie.sla.TestSLAService)
          . testMain(org.apache.oozie.action.hadoop.TestHiveMain)
          . testPigScript(org.apache.oozie.action.hadoop.TestPigMain)
          . testPig_withNullExternalID(org.apache.oozie.action.hadoop.TestPigMain)
          . testEmbeddedPigWithinPython(org.apache.oozie.action.hadoop.TestPigMain)
          . testPigScript(org.apache.oozie.action.hadoop.TestPigMainWithOldAPI)

          +1 DISTRO
          . +1 distro tarball builds with the patch

          ----------------------------
          -1 Overall result, please check the reported -1(s)

          The full output of the test-patch run is available at

          . https://builds.apache.org/job/oozie-trunk-precommit-build/2609/

          Show
          hadoopqa Hadoop QA added a comment - Testing JIRA OOZIE-2030 Cleaning local git workspace ---------------------------- +1 PATCH_APPLIES +1 CLEAN +1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . +1 the patch does not introduce any line longer than 132 . +1 the patch does adds/modifies 1 testcase(s) +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 BACKWARDS_COMPATIBILITY . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations . +1 the patch does not modify JPA files -1 TESTS . Tests run: 1703 . Tests failed: 8 . Tests errors: 6 . The patch failed the following testcases: . testForNoDuplicates(org.apache.oozie.event.TestEventGeneration) . testSamplers(org.apache.oozie.util.TestMetricsInstrumentation) . testUpdateSLA(org.apache.oozie.sla.TestSLAService) . testMain(org.apache.oozie.action.hadoop.TestHiveMain) . testPigScript(org.apache.oozie.action.hadoop.TestPigMain) . testPig_withNullExternalID(org.apache.oozie.action.hadoop.TestPigMain) . testEmbeddedPigWithinPython(org.apache.oozie.action.hadoop.TestPigMain) . testPigScript(org.apache.oozie.action.hadoop.TestPigMainWithOldAPI) +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- -1 Overall result, please check the reported -1(s) The full output of the test-patch run is available at . https://builds.apache.org/job/oozie-trunk-precommit-build/2609/
          Hide
          hadoopqa Hadoop QA added a comment -

          Testing JIRA OOZIE-2030

          Cleaning local git workspace

          ----------------------------

          +1 PATCH_APPLIES
          +1 CLEAN
          +1 RAW_PATCH_ANALYSIS
          . +1 the patch does not introduce any @author tags
          . +1 the patch does not introduce any tabs
          . +1 the patch does not introduce any trailing spaces
          . +1 the patch does not introduce any line longer than 132
          . +1 the patch does adds/modifies 1 testcase(s)
          +1 RAT
          . +1 the patch does not seem to introduce new RAT warnings
          +1 JAVADOC
          . +1 the patch does not seem to introduce new Javadoc warnings
          +1 COMPILE
          . +1 HEAD compiles
          . +1 patch compiles
          . +1 the patch does not seem to introduce new javac warnings
          +1 BACKWARDS_COMPATIBILITY
          . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations
          . +1 the patch does not modify JPA files
          -1 TESTS
          . Tests run: 1703
          . Tests failed: 2
          . Tests errors: 4

          . The patch failed the following testcases:

          . testForNoDuplicates(org.apache.oozie.event.TestEventGeneration)
          . testSamplers(org.apache.oozie.util.TestMetricsInstrumentation)

          +1 DISTRO
          . +1 distro tarball builds with the patch

          ----------------------------
          -1 Overall result, please check the reported -1(s)

          The full output of the test-patch run is available at

          . https://builds.apache.org/job/oozie-trunk-precommit-build/2615/

          Show
          hadoopqa Hadoop QA added a comment - Testing JIRA OOZIE-2030 Cleaning local git workspace ---------------------------- +1 PATCH_APPLIES +1 CLEAN +1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . +1 the patch does not introduce any line longer than 132 . +1 the patch does adds/modifies 1 testcase(s) +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 BACKWARDS_COMPATIBILITY . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations . +1 the patch does not modify JPA files -1 TESTS . Tests run: 1703 . Tests failed: 2 . Tests errors: 4 . The patch failed the following testcases: . testForNoDuplicates(org.apache.oozie.event.TestEventGeneration) . testSamplers(org.apache.oozie.util.TestMetricsInstrumentation) +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- -1 Overall result, please check the reported -1(s) The full output of the test-patch run is available at . https://builds.apache.org/job/oozie-trunk-precommit-build/2615/
          Hide
          hadoopqa Hadoop QA added a comment -

          Testing JIRA OOZIE-2030

          Cleaning local git workspace

          ----------------------------

          +1 PATCH_APPLIES
          +1 CLEAN
          +1 RAW_PATCH_ANALYSIS
          . +1 the patch does not introduce any @author tags
          . +1 the patch does not introduce any tabs
          . +1 the patch does not introduce any trailing spaces
          . +1 the patch does not introduce any line longer than 132
          . +1 the patch does adds/modifies 1 testcase(s)
          +1 RAT
          . +1 the patch does not seem to introduce new RAT warnings
          +1 JAVADOC
          . +1 the patch does not seem to introduce new Javadoc warnings
          +1 COMPILE
          . +1 HEAD compiles
          . +1 patch compiles
          . +1 the patch does not seem to introduce new javac warnings
          +1 BACKWARDS_COMPATIBILITY
          . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations
          . +1 the patch does not modify JPA files
          -1 TESTS
          . Tests run: 1703
          . Tests failed: 2
          . Tests errors: 4

          . The patch failed the following testcases:

          . testSamplers(org.apache.oozie.util.TestMetricsInstrumentation)
          . testForNoDuplicates(org.apache.oozie.event.TestEventGeneration)

          +1 DISTRO
          . +1 distro tarball builds with the patch

          ----------------------------
          -1 Overall result, please check the reported -1(s)

          The full output of the test-patch run is available at

          . https://builds.apache.org/job/oozie-trunk-precommit-build/2628/

          Show
          hadoopqa Hadoop QA added a comment - Testing JIRA OOZIE-2030 Cleaning local git workspace ---------------------------- +1 PATCH_APPLIES +1 CLEAN +1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . +1 the patch does not introduce any line longer than 132 . +1 the patch does adds/modifies 1 testcase(s) +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 BACKWARDS_COMPATIBILITY . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations . +1 the patch does not modify JPA files -1 TESTS . Tests run: 1703 . Tests failed: 2 . Tests errors: 4 . The patch failed the following testcases: . testSamplers(org.apache.oozie.util.TestMetricsInstrumentation) . testForNoDuplicates(org.apache.oozie.event.TestEventGeneration) +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- -1 Overall result, please check the reported -1(s) The full output of the test-patch run is available at . https://builds.apache.org/job/oozie-trunk-precommit-build/2628/
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Updating the final patch, Thanks Rohini Palaniswamy for review.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Updating the final patch, Thanks Rohini Palaniswamy for review.
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Updated the patch as suggested review and Rebased with latest repo.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Updated the patch as suggested review and Rebased with latest repo.
          Hide
          hadoopqa Hadoop QA added a comment -

          Testing JIRA OOZIE-2030

          Cleaning local git workspace

          ----------------------------

          +1 PATCH_APPLIES
          +1 CLEAN
          +1 RAW_PATCH_ANALYSIS
          . +1 the patch does not introduce any @author tags
          . +1 the patch does not introduce any tabs
          . +1 the patch does not introduce any trailing spaces
          . +1 the patch does not introduce any line longer than 132
          . +1 the patch does adds/modifies 1 testcase(s)
          +1 RAT
          . +1 the patch does not seem to introduce new RAT warnings
          +1 JAVADOC
          . +1 the patch does not seem to introduce new Javadoc warnings
          +1 COMPILE
          . +1 HEAD compiles
          . +1 patch compiles
          . +1 the patch does not seem to introduce new javac warnings
          +1 BACKWARDS_COMPATIBILITY
          . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations
          . +1 the patch does not modify JPA files
          -1 TESTS
          . Tests run: 1704
          . Tests failed: 3
          . Tests errors: 0

          . The patch failed the following testcases:

          . testForNoDuplicates(org.apache.oozie.event.TestEventGeneration)
          . testSamplers(org.apache.oozie.util.TestMetricsInstrumentation)
          . testUpdateSLA(org.apache.oozie.sla.TestSLAService)

          +1 DISTRO
          . +1 distro tarball builds with the patch

          ----------------------------
          -1 Overall result, please check the reported -1(s)

          The full output of the test-patch run is available at

          . https://builds.apache.org/job/oozie-trunk-precommit-build/2688/

          Show
          hadoopqa Hadoop QA added a comment - Testing JIRA OOZIE-2030 Cleaning local git workspace ---------------------------- +1 PATCH_APPLIES +1 CLEAN +1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . +1 the patch does not introduce any line longer than 132 . +1 the patch does adds/modifies 1 testcase(s) +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 BACKWARDS_COMPATIBILITY . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations . +1 the patch does not modify JPA files -1 TESTS . Tests run: 1704 . Tests failed: 3 . Tests errors: 0 . The patch failed the following testcases: . testForNoDuplicates(org.apache.oozie.event.TestEventGeneration) . testSamplers(org.apache.oozie.util.TestMetricsInstrumentation) . testUpdateSLA(org.apache.oozie.sla.TestSLAService) +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- -1 Overall result, please check the reported -1(s) The full output of the test-patch run is available at . https://builds.apache.org/job/oozie-trunk-precommit-build/2688/
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Rohini Palaniswamy,All comments have been addressed and rebased the patch, Any other thing you want me to update in the patch.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Rohini Palaniswamy ,All comments have been addressed and rebased the patch, Any other thing you want me to update in the patch.
          Hide
          rohini Rohini Palaniswamy added a comment -

          Had a minor comment on WritableUtils.writeStr(dataOutput, null) in review board. But that is fine. I will change it myself and commit it today. You do not have to upload another patch.

          Show
          rohini Rohini Palaniswamy added a comment - Had a minor comment on WritableUtils.writeStr(dataOutput, null) in review board. But that is fine. I will change it myself and commit it today. You do not have to upload another patch.
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Rohini Palaniswamy thanks for reply, I updated the patch that time only, As the change was very minor I have not uploaded in the reviewboard.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Rohini Palaniswamy thanks for reply, I updated the patch that time only, As the change was very minor I have not uploaded in the reviewboard.
          Hide
          rohini Rohini Palaniswamy added a comment -

          +1. Committed to master.

          I updated the patch that time only, As the change was very minor I have not uploaded in the reviewboard.

          Sorry. Did not notice that. Thanks for pointing out. Committed OOZIE-2030-v12.patch as is.

          Show
          rohini Rohini Palaniswamy added a comment - +1. Committed to master. I updated the patch that time only, As the change was very minor I have not uploaded in the reviewboard. Sorry. Did not notice that. Thanks for pointing out. Committed OOZIE-2030 -v12.patch as is.
          Hide
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment -

          Thanks you very much for reviewing and committing the path.

          Show
          jaydeepvishwakarma Jaydeep Vishwakarma added a comment - Thanks you very much for reviewing and committing the path.
          Hide
          rkanter Robert Kanter added a comment -

          In our testing, we ran into a situation where you could get an NPE from this change. I've filed OOZIE-2441 to fix it.

          Show
          rkanter Robert Kanter added a comment - In our testing, we ran into a situation where you could get an NPE from this change. I've filed OOZIE-2441 to fix it.
          Hide
          rkanter Robert Kanter added a comment -

          Closing issue; Oozie 4.3.0 is released.

          Show
          rkanter Robert Kanter added a comment - Closing issue; Oozie 4.3.0 is released.

            People

            • Assignee:
              jaydeepvishwakarma Jaydeep Vishwakarma
              Reporter:
              peeyushb Peeyush Bishnoi
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development