You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Ryota Egashira <eg...@yahoo-inc.com> on 2015/02/20 19:53:10 UTC
Review Request 31232: OOZIE-2146 add SLA bulk API
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31232/
-----------------------------------------------------------
Review request for oozie.
Repository: oozie-git
Description
-------
https://issues.apache.org/jira/browse/OOZIE-2146
Diffs
-----
client/src/main/java/org/apache/oozie/client/OozieClient.java e4c93cd
client/src/main/java/org/apache/oozie/client/rest/JsonTags.java b7cf0e7
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java b55cda3
core/src/main/java/org/apache/oozie/servlet/V2SLAServlet.java a0fe1b6
core/src/main/java/org/apache/oozie/sla/SLASummaryBean.java 9907dd0
core/src/test/java/org/apache/oozie/servlet/DagServletTestCase.java 48193c7
core/src/test/java/org/apache/oozie/servlet/TestV2SLAServlet.java 5f51b22
docs/src/site/twiki/DG_SLAMonitoring.twiki acf8ac1
Diff: https://reviews.apache.org/r/31232/diff/
Testing
-------
Thanks,
Ryota Egashira
Re: Review Request 31232: OOZIE-2146 add SLA bulk API
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31232/#review73307
-----------------------------------------------------------
client/src/main/java/org/apache/oozie/client/OozieClient.java
<https://reviews.apache.org/r/31232/#comment119587>
Can you rename this to FILTER_BUNDLE? so that other command can also use it.
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment119615>
Just a suggestion, this method is very big and complex to understand. Is that possible to break down in multiple functions?
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment119606>
I guess this should be .append(" AND s.expectedStartTS >= s.actualStartTS) ")
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment119608>
What if job is still running and it has missed duration. If job is running actualDuration will not be populated.
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment119609>
Can't we just use status. For end_miss and end_met, evenstatus will have correct status.
core/src/main/java/org/apache/oozie/servlet/V2SLAServlet.java
<https://reviews.apache.org/r/31232/#comment119610>
We have already defined name pattern in BulkJPAExecutor. Can you please reuse that, in that way we can avoid duplicates.
core/src/main/java/org/apache/oozie/servlet/V2SLAServlet.java
<https://reviews.apache.org/r/31232/#comment119617>
Why do we need this? Can't this be done in sql itsleft.
for DURATION_MISS,
id ( event_status=DURATION_MISS || ( actualDuration > expectedDuration))
core/src/main/java/org/apache/oozie/sla/SLASummaryBean.java
<https://reviews.apache.org/r/31232/#comment119618>
I guess we have same logic for SLA UI to populate eventStatus. Can please resuse this code to avoid duplicate code.
core/src/test/java/org/apache/oozie/servlet/TestV2SLAServlet.java
<https://reviews.apache.org/r/31232/#comment119622>
Can you also assert event_status?
core/src/test/java/org/apache/oozie/servlet/TestV2SLAServlet.java
<https://reviews.apache.org/r/31232/#comment119629>
With "event_status=DURATION_MISS;sla_status=IN_PROCESS,MET" option, you will should never get record with END_MISS.
- Purshotam Shah
On Feb. 20, 2015, 6:53 p.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31232/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2015, 6:53 p.m.)
>
>
> Review request for oozie.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-2146
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/oozie/client/OozieClient.java e4c93cd
> client/src/main/java/org/apache/oozie/client/rest/JsonTags.java b7cf0e7
> core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java b55cda3
> core/src/main/java/org/apache/oozie/servlet/V2SLAServlet.java a0fe1b6
> core/src/main/java/org/apache/oozie/sla/SLASummaryBean.java 9907dd0
> core/src/test/java/org/apache/oozie/servlet/DagServletTestCase.java 48193c7
> core/src/test/java/org/apache/oozie/servlet/TestV2SLAServlet.java 5f51b22
> docs/src/site/twiki/DG_SLAMonitoring.twiki acf8ac1
>
> Diff: https://reviews.apache.org/r/31232/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request 31232: OOZIE-2146 add SLA bulk API
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31232/#review75534
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment122673>
If one of jobId, parentId, bundle, nominal_time filters is not specified throw an error asking to specify them.
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment122704>
Do debug logging of the constructed query.
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment122675>
Can't we use the eventProcessed bit? Query will be simpler
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment122701>
CURRENT_TIMESTAMP - Isn't this database specific? We should probably add it as query param
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment122702>
Can directly put -1 here. Also expectedDuration != -1 is enough.
core/src/main/java/org/apache/oozie/servlet/V2SLAServlet.java
<https://reviews.apache.org/r/31232/#comment122677>
Let us just do .* instead of Services.get().getSystemId(). If that changes older bundles will not be matched
core/src/main/java/org/apache/oozie/servlet/V2SLAServlet.java
<https://reviews.apache.org/r/31232/#comment122679>
Get rid of the bulk and return all the new fields everytime.
core/src/test/java/org/apache/oozie/servlet/TestV2SLAServlet.java
<https://reviews.apache.org/r/31232/#comment122680>
?
docs/src/site/twiki/DG_SLAMonitoring.twiki
<https://reviews.apache.org/r/31232/#comment122682>
Remove this line
docs/src/site/twiki/DG_SLAMonitoring.twiki
<https://reviews.apache.org/r/31232/#comment122683>
Bundle Job ID or Bundle App Name. Fetches SLA information for actions of all coordinators in that bundle.
docs/src/site/twiki/DG_SLAMonitoring.twiki
<https://reviews.apache.org/r/31232/#comment122690>
with comma separation. When multiple statuses are specified, it is considered as OR.
docs/src/site/twiki/DG_SLAMonitoring.twiki
<https://reviews.apache.org/r/31232/#comment122684>
coordiantor -> coordinator
docs/src/site/twiki/DG_SLAMonitoring.twiki
<https://reviews.apache.org/r/31232/#comment122685>
Remove this example and just add new fields to old examples.
If you are keeping it change title to "All Coordinator actions in a Bundle"
docs/src/site/twiki/DG_SLAMonitoring.twiki
<https://reviews.apache.org/r/31232/#comment122688>
Should be number
docs/src/site/twiki/DG_SLAMonitoring.twiki
<https://reviews.apache.org/r/31232/#comment122689>
Remove this line. The filters should apply in all cases.
docs/src/site/twiki/DG_SLAMonitoring.twiki
<https://reviews.apache.org/r/31232/#comment122686>
Return negative value even for met.
- Rohini Palaniswamy
On Feb. 25, 2015, 7:44 p.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31232/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2015, 7:44 p.m.)
>
>
> Review request for oozie.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-2146
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/oozie/client/OozieClient.java 5de25cc
> client/src/main/java/org/apache/oozie/client/rest/JsonTags.java 1022dd7
> core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java b55cda3
> core/src/main/java/org/apache/oozie/servlet/V2SLAServlet.java 57170e1
> core/src/main/java/org/apache/oozie/sla/SLASummaryBean.java a88dcf6
> core/src/test/java/org/apache/oozie/servlet/DagServletTestCase.java 48193c7
> core/src/test/java/org/apache/oozie/servlet/TestV2SLAServlet.java 1886f48
> docs/src/site/twiki/DG_SLAMonitoring.twiki 1413945
>
> Diff: https://reviews.apache.org/r/31232/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request 31232: OOZIE-2146 add SLA bulk API
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31232/#review78536
-----------------------------------------------------------
Ship it!
Ship It!
- Rohini Palaniswamy
On April 1, 2015, 1:42 p.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31232/
> -----------------------------------------------------------
>
> (Updated April 1, 2015, 1:42 p.m.)
>
>
> Review request for oozie.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-2146
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/oozie/client/OozieClient.java 5de25cc
> client/src/main/java/org/apache/oozie/client/rest/JsonTags.java 1022dd7
> core/src/main/java/org/apache/oozie/executor/jpa/CoordinatorJobGetForUserAppnameJPAExecutor.java 3bb2f88
> core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java b55cda3
> core/src/main/java/org/apache/oozie/servlet/V2SLAServlet.java 57170e1
> core/src/main/java/org/apache/oozie/sla/SLASummaryBean.java a88dcf6
> core/src/test/java/org/apache/oozie/servlet/DagServletTestCase.java 48193c7
> core/src/test/java/org/apache/oozie/servlet/TestV2SLAServlet.java 1886f48
> docs/src/site/twiki/DG_SLAMonitoring.twiki 1413945
>
> Diff: https://reviews.apache.org/r/31232/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request 31232: OOZIE-2146 add SLA bulk API
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31232/
-----------------------------------------------------------
(Updated April 1, 2015, 1:42 p.m.)
Review request for oozie.
Changes
-------
fixed review comments
Repository: oozie-git
Description
-------
https://issues.apache.org/jira/browse/OOZIE-2146
Diffs (updated)
-----
client/src/main/java/org/apache/oozie/client/OozieClient.java 5de25cc
client/src/main/java/org/apache/oozie/client/rest/JsonTags.java 1022dd7
core/src/main/java/org/apache/oozie/executor/jpa/CoordinatorJobGetForUserAppnameJPAExecutor.java 3bb2f88
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java b55cda3
core/src/main/java/org/apache/oozie/servlet/V2SLAServlet.java 57170e1
core/src/main/java/org/apache/oozie/sla/SLASummaryBean.java a88dcf6
core/src/test/java/org/apache/oozie/servlet/DagServletTestCase.java 48193c7
core/src/test/java/org/apache/oozie/servlet/TestV2SLAServlet.java 1886f48
docs/src/site/twiki/DG_SLAMonitoring.twiki 1413945
Diff: https://reviews.apache.org/r/31232/diff/
Testing
-------
Thanks,
Ryota Egashira
Re: Review Request 31232: OOZIE-2146 add SLA bulk API
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31232/#review78177
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment126689>
Remove import
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment126686>
private
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment126690>
LOG.debug
docs/src/site/twiki/DG_SLAMonitoring.twiki
<https://reviews.apache.org/r/31232/#comment126691>
No quotes
docs/src/site/twiki/DG_SLAMonitoring.twiki
<https://reviews.apache.org/r/31232/#comment126692>
startDelay/durationDelay/endDelay values returned indicate how much delay .... (Remove Bulk API)
startDelay/durationDelay/endDelay and eventStatus also need to be added to previous examples.
core/src/main/java/org/apache/oozie/executor/jpa/CoordinatorJobGetForUserAppnameJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment126688>
private
core/src/main/java/org/apache/oozie/sla/SLASummaryBean.java
<https://reviews.apache.org/r/31232/#comment126694>
Remove boolean bulk
- Rohini Palaniswamy
On March 23, 2015, 9:25 p.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31232/
> -----------------------------------------------------------
>
> (Updated March 23, 2015, 9:25 p.m.)
>
>
> Review request for oozie.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-2146
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/oozie/client/OozieClient.java 5de25cc
> client/src/main/java/org/apache/oozie/client/rest/JsonTags.java 1022dd7
> core/src/main/java/org/apache/oozie/executor/jpa/CoordinatorJobGetForUserAppnameJPAExecutor.java 3bb2f88
> core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java b55cda3
> core/src/main/java/org/apache/oozie/servlet/V2SLAServlet.java 57170e1
> core/src/main/java/org/apache/oozie/sla/SLASummaryBean.java a88dcf6
> core/src/test/java/org/apache/oozie/servlet/DagServletTestCase.java 48193c7
> core/src/test/java/org/apache/oozie/servlet/TestV2SLAServlet.java 1886f48
> docs/src/site/twiki/DG_SLAMonitoring.twiki 1413945
>
> Diff: https://reviews.apache.org/r/31232/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request 31232: OOZIE-2146 add SLA bulk API
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31232/
-----------------------------------------------------------
(Updated March 23, 2015, 9:25 p.m.)
Review request for oozie.
Changes
-------
modified patch based on review comments
Repository: oozie-git
Description
-------
https://issues.apache.org/jira/browse/OOZIE-2146
Diffs (updated)
-----
client/src/main/java/org/apache/oozie/client/OozieClient.java 5de25cc
client/src/main/java/org/apache/oozie/client/rest/JsonTags.java 1022dd7
core/src/main/java/org/apache/oozie/executor/jpa/CoordinatorJobGetForUserAppnameJPAExecutor.java 3bb2f88
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java b55cda3
core/src/main/java/org/apache/oozie/servlet/V2SLAServlet.java 57170e1
core/src/main/java/org/apache/oozie/sla/SLASummaryBean.java a88dcf6
core/src/test/java/org/apache/oozie/servlet/DagServletTestCase.java 48193c7
core/src/test/java/org/apache/oozie/servlet/TestV2SLAServlet.java 1886f48
docs/src/site/twiki/DG_SLAMonitoring.twiki 1413945
Diff: https://reviews.apache.org/r/31232/diff/
Testing
-------
Thanks,
Ryota Egashira
Re: Review Request 31232: OOZIE-2146 add SLA bulk API
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31232/
-----------------------------------------------------------
(Updated Feb. 25, 2015, 7:44 p.m.)
Review request for oozie.
Changes
-------
fixed review comments
Repository: oozie-git
Description
-------
https://issues.apache.org/jira/browse/OOZIE-2146
Diffs (updated)
-----
client/src/main/java/org/apache/oozie/client/OozieClient.java 5de25cc
client/src/main/java/org/apache/oozie/client/rest/JsonTags.java 1022dd7
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java b55cda3
core/src/main/java/org/apache/oozie/servlet/V2SLAServlet.java 57170e1
core/src/main/java/org/apache/oozie/sla/SLASummaryBean.java a88dcf6
core/src/test/java/org/apache/oozie/servlet/DagServletTestCase.java 48193c7
core/src/test/java/org/apache/oozie/servlet/TestV2SLAServlet.java 1886f48
docs/src/site/twiki/DG_SLAMonitoring.twiki 1413945
Diff: https://reviews.apache.org/r/31232/diff/
Testing
-------
Thanks,
Ryota Egashira