You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Pallavi Rao <pa...@inmobi.com> on 2016/01/12 11:14:13 UTC
Review Request 42188: FALCON-1742 Implement instance summary api for
native scheduler
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42188/
-----------------------------------------------------------
Review request for Falcon.
Bugs: FALCON-1742
https://issues.apache.org/jira/browse/FALCON-1742
Repository: falcon-git
Description
-------
This was not implemented.
Diffs
-----
client/src/main/java/org/apache/falcon/resource/EntitySummaryResult.java 4a885ec
scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java bffdb0b
unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java a82cf03
webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java b1c8ce0
Diff: https://reviews.apache.org/r/42188/diff/
Testing
-------
Added IT and did manual testing.
Thanks,
Pallavi Rao
Re: Review Request 42188: FALCON-1742 Implement instance summary api
for native scheduler
Posted by Ajay Yadava <aj...@gmail.com>.
> On Jan. 18, 2016, 11:01 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java, line 414
> > <https://reviews.apache.org/r/42188/diff/1/?file=1194070#file1194070line414>
> >
> > Shouldn't this be outside for loop?
>
> Pallavi Rao wrote:
> In this implementation, I'm retrieving instances per state, rather than retrieve all instances and sort and count in memory.
>
> Deepak Barr wrote:
> states variable is different for different iterations. Should be OK.
>
> Ajay Yadava wrote:
> The ordering can be done in db query, that way all invocations of the query will result in sorted result and we will not need to make multiple db calls. That won't work?
>
> Pallavi Rao wrote:
> Yeah.. it will sort, but, I'll still have to iterate over the entire result set or issue a group command. Also, the result set can be large. If it is limited to a single state, it gets returned faster. It requires additional changes to the StateStore layer. Hence, the simplistic approach.
So in this approach we will fire approximately 8 queries(1 corresponding to each state), we will fetch all the columns, convert them to POJOs whereas we need just count. What I am suggesting is just fire 1 query which will group by state and return the count corresponding to each state and this will be several times faster and simpler. Another difference is that for a large range current approach may cause memory issues(and severe performance degradation) as it fetches all columns and all rows whereas the suggested approach is immune to such cases.
- Ajay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42188/#review114982
-----------------------------------------------------------
On Jan. 12, 2016, 10:14 a.m., Pallavi Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42188/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 10:14 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1742
> https://issues.apache.org/jira/browse/FALCON-1742
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> This was not implemented.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/EntitySummaryResult.java 4a885ec
> scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java bffdb0b
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java a82cf03
> webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java b1c8ce0
>
> Diff: https://reviews.apache.org/r/42188/diff/
>
>
> Testing
> -------
>
> Added IT and did manual testing.
>
>
> Thanks,
>
> Pallavi Rao
>
>
Re: Review Request 42188: FALCON-1742 Implement instance summary api
for native scheduler
Posted by Deepak Barr <de...@gmail.com>.
> On Jan. 18, 2016, 11:01 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java, line 414
> > <https://reviews.apache.org/r/42188/diff/1/?file=1194070#file1194070line414>
> >
> > Shouldn't this be outside for loop?
>
> Pallavi Rao wrote:
> In this implementation, I'm retrieving instances per state, rather than retrieve all instances and sort and count in memory.
states variable is different for different iterations. Should be OK.
- Deepak
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42188/#review114982
-----------------------------------------------------------
On Jan. 12, 2016, 10:14 a.m., Pallavi Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42188/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 10:14 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1742
> https://issues.apache.org/jira/browse/FALCON-1742
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> This was not implemented.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/EntitySummaryResult.java 4a885ec
> scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java bffdb0b
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java a82cf03
> webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java b1c8ce0
>
> Diff: https://reviews.apache.org/r/42188/diff/
>
>
> Testing
> -------
>
> Added IT and did manual testing.
>
>
> Thanks,
>
> Pallavi Rao
>
>
Re: Review Request 42188: FALCON-1742 Implement instance summary api
for native scheduler
Posted by Pallavi Rao <pa...@inmobi.com>.
> On Jan. 18, 2016, 11:01 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java, line 414
> > <https://reviews.apache.org/r/42188/diff/1/?file=1194070#file1194070line414>
> >
> > Shouldn't this be outside for loop?
>
> Pallavi Rao wrote:
> In this implementation, I'm retrieving instances per state, rather than retrieve all instances and sort and count in memory.
>
> Deepak Barr wrote:
> states variable is different for different iterations. Should be OK.
>
> Ajay Yadava wrote:
> The ordering can be done in db query, that way all invocations of the query will result in sorted result and we will not need to make multiple db calls. That won't work?
Yeah.. it will sort, but, I'll still have to iterate over the entire result set or issue a group command. Also, the result set can be large. If it is limited to a single state, it gets returned faster. It requires additional changes to the StateStore layer. Hence, the simplistic approach.
- Pallavi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42188/#review114982
-----------------------------------------------------------
On Jan. 12, 2016, 10:14 a.m., Pallavi Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42188/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 10:14 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1742
> https://issues.apache.org/jira/browse/FALCON-1742
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> This was not implemented.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/EntitySummaryResult.java 4a885ec
> scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java bffdb0b
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java a82cf03
> webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java b1c8ce0
>
> Diff: https://reviews.apache.org/r/42188/diff/
>
>
> Testing
> -------
>
> Added IT and did manual testing.
>
>
> Thanks,
>
> Pallavi Rao
>
>
Re: Review Request 42188: FALCON-1742 Implement instance summary api
for native scheduler
Posted by Pallavi Rao <pa...@inmobi.com>.
> On Jan. 18, 2016, 11:01 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java, line 414
> > <https://reviews.apache.org/r/42188/diff/1/?file=1194070#file1194070line414>
> >
> > Shouldn't this be outside for loop?
In this implementation, I'm retrieving instances per state, rather than retrieve all instances and sort and count in memory.
- Pallavi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42188/#review114982
-----------------------------------------------------------
On Jan. 12, 2016, 10:14 a.m., Pallavi Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42188/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 10:14 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1742
> https://issues.apache.org/jira/browse/FALCON-1742
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> This was not implemented.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/EntitySummaryResult.java 4a885ec
> scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java bffdb0b
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java a82cf03
> webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java b1c8ce0
>
> Diff: https://reviews.apache.org/r/42188/diff/
>
>
> Testing
> -------
>
> Added IT and did manual testing.
>
>
> Thanks,
>
> Pallavi Rao
>
>
Re: Review Request 42188: FALCON-1742 Implement instance summary api
for native scheduler
Posted by Ajay Yadava <aj...@gmail.com>.
> On Jan. 18, 2016, 11:01 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java, line 414
> > <https://reviews.apache.org/r/42188/diff/1/?file=1194070#file1194070line414>
> >
> > Shouldn't this be outside for loop?
>
> Pallavi Rao wrote:
> In this implementation, I'm retrieving instances per state, rather than retrieve all instances and sort and count in memory.
>
> Deepak Barr wrote:
> states variable is different for different iterations. Should be OK.
The ordering can be done in db query, that way all invocations of the query will result in sorted result and we will not need to make multiple db calls. That won't work?
- Ajay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42188/#review114982
-----------------------------------------------------------
On Jan. 12, 2016, 10:14 a.m., Pallavi Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42188/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 10:14 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1742
> https://issues.apache.org/jira/browse/FALCON-1742
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> This was not implemented.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/EntitySummaryResult.java 4a885ec
> scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java bffdb0b
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java a82cf03
> webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java b1c8ce0
>
> Diff: https://reviews.apache.org/r/42188/diff/
>
>
> Testing
> -------
>
> Added IT and did manual testing.
>
>
> Thanks,
>
> Pallavi Rao
>
>
Re: Review Request 42188: FALCON-1742 Implement instance summary api
for native scheduler
Posted by Pallavi Rao <pa...@inmobi.com>.
> On Jan. 18, 2016, 11:01 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java, line 414
> > <https://reviews.apache.org/r/42188/diff/1/?file=1194070#file1194070line414>
> >
> > Shouldn't this be outside for loop?
>
> Pallavi Rao wrote:
> In this implementation, I'm retrieving instances per state, rather than retrieve all instances and sort and count in memory.
>
> Deepak Barr wrote:
> states variable is different for different iterations. Should be OK.
>
> Ajay Yadava wrote:
> The ordering can be done in db query, that way all invocations of the query will result in sorted result and we will not need to make multiple db calls. That won't work?
>
> Pallavi Rao wrote:
> Yeah.. it will sort, but, I'll still have to iterate over the entire result set or issue a group command. Also, the result set can be large. If it is limited to a single state, it gets returned faster. It requires additional changes to the StateStore layer. Hence, the simplistic approach.
>
> Ajay Yadava wrote:
> So in this approach we will fire approximately 8 queries(1 corresponding to each state), we will fetch all the columns, convert them to POJOs whereas we need just count. What I am suggesting is just fire 1 query which will group by state and return the count corresponding to each state and this will be several times faster and simpler. Another difference is that for a large range current approach may cause memory issues(and severe performance degradation) as it fetches all columns and all rows whereas the suggested approach is immune to such cases.
Yep. Makes sense. It just wasteful and can cause issues. Will address
- Pallavi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42188/#review114982
-----------------------------------------------------------
On Jan. 12, 2016, 10:14 a.m., Pallavi Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42188/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 10:14 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1742
> https://issues.apache.org/jira/browse/FALCON-1742
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> This was not implemented.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/EntitySummaryResult.java 4a885ec
> scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java bffdb0b
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java a82cf03
> webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java b1c8ce0
>
> Diff: https://reviews.apache.org/r/42188/diff/
>
>
> Testing
> -------
>
> Added IT and did manual testing.
>
>
> Thanks,
>
> Pallavi Rao
>
>
Re: Review Request 42188: FALCON-1742 Implement instance summary api
for native scheduler
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42188/#review114982
-----------------------------------------------------------
scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java (line 414)
<https://reviews.apache.org/r/42188/#comment175827>
Shouldn't this be outside for loop?
- Ajay Yadava
On Jan. 12, 2016, 10:14 a.m., Pallavi Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42188/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 10:14 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1742
> https://issues.apache.org/jira/browse/FALCON-1742
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> This was not implemented.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/EntitySummaryResult.java 4a885ec
> scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java bffdb0b
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java a82cf03
> webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java b1c8ce0
>
> Diff: https://reviews.apache.org/r/42188/diff/
>
>
> Testing
> -------
>
> Added IT and did manual testing.
>
>
> Thanks,
>
> Pallavi Rao
>
>
Re: Review Request 42188: FALCON-1742 Implement instance summary api
for native scheduler
Posted by Pallavi Rao <pa...@inmobi.com>.
> On Jan. 18, 2016, 12:12 p.m., Praveen Adlakha wrote:
> > unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java, line 330
> > <https://reviews.apache.org/r/42188/diff/1/?file=1194071#file1194071line330>
> >
> > same here.
Will change.
- Pallavi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42188/#review114997
-----------------------------------------------------------
On Jan. 12, 2016, 10:14 a.m., Pallavi Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42188/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 10:14 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1742
> https://issues.apache.org/jira/browse/FALCON-1742
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> This was not implemented.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/EntitySummaryResult.java 4a885ec
> scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java bffdb0b
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java a82cf03
> webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java b1c8ce0
>
> Diff: https://reviews.apache.org/r/42188/diff/
>
>
> Testing
> -------
>
> Added IT and did manual testing.
>
>
> Thanks,
>
> Pallavi Rao
>
>
Re: Review Request 42188: FALCON-1742 Implement instance summary api
for native scheduler
Posted by Praveen Adlakha <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42188/#review114997
-----------------------------------------------------------
unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java (line 327)
<https://reviews.apache.org/r/42188/#comment175856>
Shouldn't we use StringUtils.isBlank()?
unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java (line 330)
<https://reviews.apache.org/r/42188/#comment175857>
same here.
- Praveen Adlakha
On Jan. 12, 2016, 10:14 a.m., Pallavi Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42188/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 10:14 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1742
> https://issues.apache.org/jira/browse/FALCON-1742
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> This was not implemented.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/EntitySummaryResult.java 4a885ec
> scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java bffdb0b
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java a82cf03
> webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java b1c8ce0
>
> Diff: https://reviews.apache.org/r/42188/diff/
>
>
> Testing
> -------
>
> Added IT and did manual testing.
>
>
> Thanks,
>
> Pallavi Rao
>
>
Re: Review Request 42188: FALCON-1742 Implement instance summary api
for native scheduler
Posted by Pallavi Rao <pa...@inmobi.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42188/
-----------------------------------------------------------
(Updated Jan. 19, 2016, 8:50 a.m.)
Review request for Falcon.
Changes
-------
Addressed comments.
Bugs: FALCON-1742
https://issues.apache.org/jira/browse/FALCON-1742
Repository: falcon-git
Description
-------
This was not implemented.
Diffs (updated)
-----
client/src/main/java/org/apache/falcon/resource/EntitySummaryResult.java 4a885ec
scheduler/src/main/java/org/apache/falcon/state/store/InMemoryStateStore.java c4ced46
scheduler/src/main/java/org/apache/falcon/state/store/InstanceStateStore.java 8ce8fa0
scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java 3def14a
scheduler/src/main/java/org/apache/falcon/state/store/jdbc/InstanceBean.java 5ed3ccd
scheduler/src/main/java/org/apache/falcon/state/store/jdbc/JDBCStateStore.java e898247
scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java bffdb0b
scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java bb8ff61
unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java a82cf03
webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java b1c8ce0
Diff: https://reviews.apache.org/r/42188/diff/
Testing
-------
Added IT and did manual testing.
Thanks,
Pallavi Rao
Re: Review Request 42188: FALCON-1742 Implement instance summary api
for native scheduler
Posted by Pallavi Rao <pa...@inmobi.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42188/#review114985
-----------------------------------------------------------
- Pallavi Rao
On Jan. 12, 2016, 10:14 a.m., Pallavi Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42188/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 10:14 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1742
> https://issues.apache.org/jira/browse/FALCON-1742
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> This was not implemented.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/EntitySummaryResult.java 4a885ec
> scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java bffdb0b
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java a82cf03
> webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java b1c8ce0
>
> Diff: https://reviews.apache.org/r/42188/diff/
>
>
> Testing
> -------
>
> Added IT and did manual testing.
>
>
> Thanks,
>
> Pallavi Rao
>
>
Re: Review Request 42188: FALCON-1742 Implement instance summary api
for native scheduler
Posted by Pallavi Rao <pa...@inmobi.com>.
> On Jan. 18, 2016, 6:49 p.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java, line 399
> > <https://reviews.apache.org/r/42188/diff/1/?file=1194070#file1194070line399>
> >
> > What is the advantage of calling getIncludedClusters here? It seems that it will always return a blank list in this case with no side effects either?
None. Just remnant of copy-paste from another method :-). Will remove.
- Pallavi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42188/#review115042
-----------------------------------------------------------
On Jan. 12, 2016, 10:14 a.m., Pallavi Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42188/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 10:14 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1742
> https://issues.apache.org/jira/browse/FALCON-1742
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> This was not implemented.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/EntitySummaryResult.java 4a885ec
> scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java bffdb0b
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java a82cf03
> webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java b1c8ce0
>
> Diff: https://reviews.apache.org/r/42188/diff/
>
>
> Testing
> -------
>
> Added IT and did manual testing.
>
>
> Thanks,
>
> Pallavi Rao
>
>
Re: Review Request 42188: FALCON-1742 Implement instance summary api
for native scheduler
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42188/#review115042
-----------------------------------------------------------
scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java (line 399)
<https://reviews.apache.org/r/42188/#comment175887>
What is the advantage of calling getIncludedClusters here? It seems that it will always return a blank list in this case with no side effects either?
- Ajay Yadava
On Jan. 12, 2016, 10:14 a.m., Pallavi Rao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42188/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 10:14 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1742
> https://issues.apache.org/jira/browse/FALCON-1742
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> This was not implemented.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/EntitySummaryResult.java 4a885ec
> scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java bffdb0b
> unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java a82cf03
> webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java b1c8ce0
>
> Diff: https://reviews.apache.org/r/42188/diff/
>
>
> Testing
> -------
>
> Added IT and did manual testing.
>
>
> Thanks,
>
> Pallavi Rao
>
>