You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Rohini Palaniswamy <ro...@gmail.com> on 2015/11/30 06:06:17 UTC

Re: Review Request 35379: OOZIE-2030 Configuration properties from global section is not getting set in Hadoop job conf when using sub-workflow action in Oozie workflow.xml

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35379/#review108278
-----------------------------------------------------------


Like that the logic is lot more concise than the previous versions and you have made good use of changes in OOZIE-2187. But would like optimization to avoid cost of serializing GlobalSectionData for all workflows which do not even have sub-workflows and also the deserialization of GlobalSectionData done for every action again for all workflows which are unnecessary. Suggested changes below which will serialize global section into jobconf only when there is sub-workflow with propagate-configuration. Also deserialization of GlobalSectionData from jobconf is done only once in the beginning.


core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowAppParser.java (line 431)
<https://reviews.apache.org/r/35379/#comment167645>

    GlobalSectionData gData = jobConf.get(OOZIE_GLOBAL) == null ? null : getGlobalFromString(jobConf.get(OOZIE_GLOBAL))
    boolean serializedGlobalConf = false;



core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowAppParser.java (lines 474 - 477)
<https://reviews.apache.org/r/35379/#comment167632>

    Instead of propagateGlobalConf = true; can we do
    
    if (!serializedGlobalConf  && elem.getName().equals(SubWorkflowActionExecutor.ACTION_TYPE) &&
                                    elem.getChild(("propagate-configuration"), ns) != null) {
    serializedGlobalConf = true;                        jobConf.set(OOZIE_GLOBAL, getGlobalString(gData));
                            }
    eActionConf = elem;                       handleDefaultsAndGlobal(gData, configDefault, elem);     
    
    here instead of in eNode.getName().equals(GLOBAL) section and remove
    
    if (!propagateGlobalConf) {
                jobConf.unset(OOZIE_GLOBAL);
            }
            
    serializedGlobalConf is to avoid serializing multiple times if there is more than one sub-workflow which is the case most of the time.



core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowAppParser.java (lines 503 - 507)
<https://reviews.apache.org/r/35379/#comment167648>

    if(jobConf.get(OOZIE_GLOBAL) != null) {
     handleDefaultsAndGlobal(gData, null, eNode);
    }
    gData = parseGlobalSection(ns, eNode);
    
    gData = parseGlobalSection(ns, eNode); at the beginning before if block needs to be removed. Parse gData only once after the if block.



core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowAppParser.java (line 551)
<https://reviews.apache.org/r/35379/#comment167630>

    Can you compress before base64 encoding? It is compressed before storing to database. But in some recent issues with hcat we found that conf compresses lot better before base64 encoding than after it. So compressing here should also be good.
    
    Deflater def = new Deflater();
    oos = new DataOutputStream(new DeflaterOutputStream(baos, def));



core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowAppParser.java (lines 643 - 646)
<https://reviews.apache.org/r/35379/#comment167629>

    Make it an empty constructor. null is the default value with the variable declaration. If you want it to be explicitly null, assign it to null in the variable declaration itself



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java (line 709)
<https://reviews.apache.org/r/35379/#comment167631>

    Shouldn't foo3 be actionconf here also?


- Rohini Palaniswamy


On July 21, 2015, 10:59 a.m., Jaydeep Vishwakarma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35379/
> -----------------------------------------------------------
> 
> (Updated July 21, 2015, 10:59 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2030
>     https://issues.apache.org/jira/browse/OOZIE-2030
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> Forwarding global conf from workflow to action when action is subworkflow, 
> subworkflow storing it to property and forwarding as property. 
>  LiteworkflowAppParser handling all condition and ensuring child level pass of global conf.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/oozie/action/oozie/SubWorkflowActionExecutor.java 854d621 
>   core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowAppParser.java d3a6523 
>   core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java 9ab897a 
> 
> Diff: https://reviews.apache.org/r/35379/diff/
> 
> 
> Testing
> -------
> 
> yes
> 
> 
> Thanks,
> 
> Jaydeep Vishwakarma
> 
>


Re: Review Request 35379: OOZIE-2030 Configuration properties from global section is not getting set in Hadoop job conf when using sub-workflow action in Oozie workflow.xml

Posted by Jaydeep Vishwakarma <ja...@gmail.com>.

> On Nov. 30, 2015, 5:06 a.m., Rohini Palaniswamy wrote:
> > core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java, line 709
> > <https://reviews.apache.org/r/35379/diff/7/?file=1017730#file1017730line709>
> >
> >     Shouldn't foo3 be actionconf here also?

No it will be foo3 only and it is independent to this patch. Configuration tag has no code to merge with action conf. But both are independently exists in some form to use. In Action executor actionconf have "foo3=foo3" and configuration conf "foo3=actionconf" can be access through context.getworkflow.getConf/context.getprotoconf , And for same reason I have mentioned that value in test case.  I will fix it as separate patch as it has no relation with global conf.


- Jaydeep


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35379/#review108278
-----------------------------------------------------------


On Nov. 30, 2015, 7:50 p.m., Jaydeep Vishwakarma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35379/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2015, 7:50 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2030
>     https://issues.apache.org/jira/browse/OOZIE-2030
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> Forwarding global conf from workflow to action when action is subworkflow, 
> subworkflow storing it to property and forwarding as property. 
>  LiteworkflowAppParser handling all condition and ensuring child level pass of global conf.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/oozie/action/oozie/SubWorkflowActionExecutor.java 854d621 
>   core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowAppParser.java d3a6523 
>   core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java 9ab897a 
> 
> Diff: https://reviews.apache.org/r/35379/diff/
> 
> 
> Testing
> -------
> 
> yes
> 
> 
> Thanks,
> 
> Jaydeep Vishwakarma
> 
>