You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Virag Kothari <vi...@yahoo-inc.com> on 2013/01/08 00:52:15 UTC

Review Request: OOZIE-1161: Remove unnecessary db updates for some of the blobs like missing_dependencies' of Coordinator Action

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

Review request for oozie.


Description
-------

Patch for hcat-intre branch.
As OOZIE-1156 is not yet committed, some of its changes related to CoordPushX are seen in this diff


This addresses bug OOZIE-1161.
    https://issues.apache.org/jira/browse/OOZIE-1161


Diffs
-----

  branches/hcat-intre/core/src/main/java/org/apache/oozie/CoordinatorActionBean.java 1428524 
  branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java 1428524 
  branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java 1428524 
  branches/hcat-intre/core/src/main/java/org/apache/oozie/executor/jpa/CoordActionUpdateForModifiedTimeJPAExecutor.java PRE-CREATION 
  branches/hcat-intre/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordActionUpdateForModifiedTimeJPAExecutor.java PRE-CREATION 

Diff: https://reviews.apache.org/r/8872/diff/


Testing
-------


Thanks,

Virag Kothari


Re: Review Request: OOZIE-1161: Remove unnecessary db updates for some of the blobs like missing_dependencies' of Coordinator Action

Posted by Virag Kothari <vi...@yahoo-inc.com>.

> On Jan. 8, 2013, 7:58 p.m., Mona Chitnis wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java, line 101
> > <https://reviews.apache.org/r/8872/diff/2/?file=245493#file245493line101>
> >
> >     can availableDepList be renamed? These are still a from of missing deps right (since initialized using pushDepList)

discussed offline..not required as though its initialized with pushDepList, it becomes availableList after missdeps are removed


> On Jan. 8, 2013, 7:58 p.m., Mona Chitnis wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java, line 106
> > <https://reviews.apache.org/r/8872/diff/2/?file=245493#file245493line106>
> >
> >     plz add a comment to explain what happens in this if block. It is unclear

will do before committing


> On Jan. 8, 2013, 7:58 p.m., Mona Chitnis wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java, line 213
> > <https://reviews.apache.org/r/8872/diff/2/?file=245493#file245493line213>
> >
> >     I dont see how wise it is to lock on job-id, since the private method getMissingDependencies invokes URIHandler.exists which is a remote call and is time-consuming

discussed offline..lock on job-id will allow throttling on the coordpushcheckX command as only one action per job can execute the command at a time..
also hcat will not be polled very often if job-id is kept as the lock


- Virag


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


On Jan. 7, 2013, 11:52 p.m., Virag Kothari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8872/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2013, 11:52 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> Patch for hcat-intre branch.
> As OOZIE-1156 is not yet committed, some of its changes related to CoordPushX are seen in this diff
> 
> 
> This addresses bug OOZIE-1161.
>     https://issues.apache.org/jira/browse/OOZIE-1161
> 
> 
> Diffs
> -----
> 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/CoordinatorActionBean.java 1428524 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java 1428524 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java 1428524 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/executor/jpa/CoordActionUpdateForModifiedTimeJPAExecutor.java PRE-CREATION 
>   branches/hcat-intre/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordActionUpdateForModifiedTimeJPAExecutor.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8872/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Virag Kothari
> 
>


Re: Review Request: OOZIE-1161: Remove unnecessary db updates for some of the blobs like missing_dependencies' of Coordinator Action

Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8872/#review15152
-----------------------------------------------------------



branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java
<https://reviews.apache.org/r/8872/#comment32741>

    can availableDepList be renamed? These are still a from of missing deps right (since initialized using pushDepList)



branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java
<https://reviews.apache.org/r/8872/#comment32740>

    plz add a comment to explain what happens in this if block. It is unclear



branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java
<https://reviews.apache.org/r/8872/#comment32742>

    I dont see how wise it is to lock on job-id, since the private method getMissingDependencies invokes URIHandler.exists which is a remote call and is time-consuming


- Mona Chitnis


On Jan. 7, 2013, 11:52 p.m., Virag Kothari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8872/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2013, 11:52 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> Patch for hcat-intre branch.
> As OOZIE-1156 is not yet committed, some of its changes related to CoordPushX are seen in this diff
> 
> 
> This addresses bug OOZIE-1161.
>     https://issues.apache.org/jira/browse/OOZIE-1161
> 
> 
> Diffs
> -----
> 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/CoordinatorActionBean.java 1428524 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java 1428524 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java 1428524 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/executor/jpa/CoordActionUpdateForModifiedTimeJPAExecutor.java PRE-CREATION 
>   branches/hcat-intre/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordActionUpdateForModifiedTimeJPAExecutor.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8872/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Virag Kothari
> 
>


Re: Review Request: OOZIE-1161: Remove unnecessary db updates for some of the blobs like missing_dependencies' of Coordinator Action

Posted by Virag Kothari <vi...@yahoo-inc.com>.

> On Jan. 8, 2013, midnight, Rohini Palaniswamy wrote:
> > branches/hcat-intre/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordActionUpdateForModifiedTimeJPAExecutor.java, line 42
> > <https://reviews.apache.org/r/8872/diff/2/?file=245495#file245495line42>
> >
> >     Is this required? Can be removed

discussed offline..required for clean up of tables from previous test cases 


- Virag


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


On Jan. 7, 2013, 11:52 p.m., Virag Kothari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8872/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2013, 11:52 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> Patch for hcat-intre branch.
> As OOZIE-1156 is not yet committed, some of its changes related to CoordPushX are seen in this diff
> 
> 
> This addresses bug OOZIE-1161.
>     https://issues.apache.org/jira/browse/OOZIE-1161
> 
> 
> Diffs
> -----
> 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/CoordinatorActionBean.java 1428524 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java 1428524 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java 1428524 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/executor/jpa/CoordActionUpdateForModifiedTimeJPAExecutor.java PRE-CREATION 
>   branches/hcat-intre/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordActionUpdateForModifiedTimeJPAExecutor.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8872/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Virag Kothari
> 
>


Re: Review Request: OOZIE-1161: Remove unnecessary db updates for some of the blobs like missing_dependencies' of Coordinator Action

Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8872/#review15130
-----------------------------------------------------------

Ship it!



branches/hcat-intre/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordActionUpdateForModifiedTimeJPAExecutor.java
<https://reviews.apache.org/r/8872/#comment32723>

    Is this required? Can be removed


- Rohini Palaniswamy


On Jan. 7, 2013, 11:52 p.m., Virag Kothari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8872/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2013, 11:52 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> Patch for hcat-intre branch.
> As OOZIE-1156 is not yet committed, some of its changes related to CoordPushX are seen in this diff
> 
> 
> This addresses bug OOZIE-1161.
>     https://issues.apache.org/jira/browse/OOZIE-1161
> 
> 
> Diffs
> -----
> 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/CoordinatorActionBean.java 1428524 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java 1428524 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java 1428524 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/executor/jpa/CoordActionUpdateForModifiedTimeJPAExecutor.java PRE-CREATION 
>   branches/hcat-intre/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordActionUpdateForModifiedTimeJPAExecutor.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8872/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Virag Kothari
> 
>


Re: Review Request: OOZIE-1161: Remove unnecessary db updates for some of the blobs like missing_dependencies' of Coordinator Action

Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8872/#review15163
-----------------------------------------------------------

Ship it!


Ship It!

- Mona Chitnis


On Jan. 7, 2013, 11:52 p.m., Virag Kothari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8872/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2013, 11:52 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> Patch for hcat-intre branch.
> As OOZIE-1156 is not yet committed, some of its changes related to CoordPushX are seen in this diff
> 
> 
> This addresses bug OOZIE-1161.
>     https://issues.apache.org/jira/browse/OOZIE-1161
> 
> 
> Diffs
> -----
> 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/CoordinatorActionBean.java 1428524 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java 1428524 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java 1428524 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/executor/jpa/CoordActionUpdateForModifiedTimeJPAExecutor.java PRE-CREATION 
>   branches/hcat-intre/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordActionUpdateForModifiedTimeJPAExecutor.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/8872/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Virag Kothari
> 
>