You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Paul Isaychuk <pi...@hortonworks.com> on 2015/12/28 16:42:24 UTC

Review Request 41742: [FALCON-1697] Stabilization of scenarios which are based on instances lifecycle

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

Review request for Falcon.


Bugs: FALCON-1697
    https://issues.apache.org/jira/browse/FALCON-1697


Repository: falcon-git


Description
-------

Fixes related to stabilization of tests which are based on instance lifecycle by adding timeouts, separating data required for different instances etc, optimization scenario.


Diffs
-----

  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 3d05ae9 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java 6493133 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceSuspendTest.java f673314 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java 7ad4c8e 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListProcessInstancesTest.java 43bdd87 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/EntityPageTest.java 4ad775e 

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


Testing
-------

Testing done.


Thanks,

Paul Isaychuk


Re: Review Request 41742: [FALCON-1697] Stabilization of scenarios which are based on instances lifecycle

Posted by Paul Isaychuk <pi...@hortonworks.com>.

> On Jan. 4, 2016, 12:40 p.m., PRAGYA MITTAL wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java, line 96
> > <https://reviews.apache.org/r/41742/diff/2/?file=1176903#file1176903line96>
> >
> >     One concern please, why did we change this test case. Existing combination is fulfilling all the scenarios as far as possible.

While 2 killed instances are enough to test the same scenario, 6 killed instances take much more time (each instances needs to be runnung, and then killed). Reducing time on this point allows us to change @BeforeClass to @BeforeMethod, which renews test precondition each time making it more stable (e.g. it guarantees that no instances are able to succeed while we need it running).


> On Jan. 4, 2016, 12:40 p.m., PRAGYA MITTAL wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java, line 74
> > <https://reviews.apache.org/r/41742/diff/2/?file=1176903#file1176903line74>
> >
> >     Why 2010. Consider giving a date of 2015.

Copy-past one. Will address it in another jira.


> On Jan. 4, 2016, 12:40 p.m., PRAGYA MITTAL wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java, line 244
> > <https://reviews.apache.org/r/41742/diff/2/?file=1176903#file1176903line244>
> >
> >     Minor spelling mistake :
> >     check on order

Thanks. Will fix it in another jira.


> On Jan. 4, 2016, 12:40 p.m., PRAGYA MITTAL wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java, line 261
> > <https://reviews.apache.org/r/41742/diff/2/?file=1176903#file1176903line261>
> >
> >     Does it mean we intend to list feed instances using 'list' api in this test case ?

Agree, comment is misleading. Will fix it in another jira.


- Paul


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


On Dec. 28, 2015, 4:05 p.m., Paul Isaychuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41742/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2015, 4:05 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1697
>     https://issues.apache.org/jira/browse/FALCON-1697
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Fixes related to stabilization of tests which are based on instance lifecycle by adding timeouts, separating data required for different instances etc, optimization scenario.
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 3d05ae9 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java 6493133 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceSuspendTest.java f673314 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java 7ad4c8e 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListProcessInstancesTest.java 43bdd87 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/EntityPageTest.java 4ad775e 
> 
> Diff: https://reviews.apache.org/r/41742/diff/
> 
> 
> Testing
> -------
> 
> Testing done.
> 
> 
> Thanks,
> 
> Paul Isaychuk
> 
>


Re: Review Request 41742: [FALCON-1697] Stabilization of scenarios which are based on instances lifecycle

Posted by PRAGYA MITTAL <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41742/#review112561
-----------------------------------------------------------



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java (line 72)
<https://reviews.apache.org/r/41742/#comment173056>

    Why 2010. Consider giving a date of 2015.



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java (line 90)
<https://reviews.apache.org/r/41742/#comment173059>

    One concern please, why did we change this test case. Existing combination is fulfilling all the scenarios as far as possible.



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java (line 238)
<https://reviews.apache.org/r/41742/#comment173057>

    Minor spelling mistake :
    check on order



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java (line 255)
<https://reviews.apache.org/r/41742/#comment173058>

    Does it mean we intend to list feed instances using 'list' api in this test case ?


- PRAGYA MITTAL


On Dec. 28, 2015, 4:05 p.m., Paul Isaychuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41742/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2015, 4:05 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1697
>     https://issues.apache.org/jira/browse/FALCON-1697
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Fixes related to stabilization of tests which are based on instance lifecycle by adding timeouts, separating data required for different instances etc, optimization scenario.
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 3d05ae9 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java 6493133 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceSuspendTest.java f673314 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java 7ad4c8e 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListProcessInstancesTest.java 43bdd87 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/EntityPageTest.java 4ad775e 
> 
> Diff: https://reviews.apache.org/r/41742/diff/
> 
> 
> Testing
> -------
> 
> Testing done.
> 
> 
> Thanks,
> 
> Paul Isaychuk
> 
>


Re: Review Request 41742: [FALCON-1697] Stabilization of scenarios which are based on instances lifecycle

Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41742/#review112558
-----------------------------------------------------------

Ship it!


Ship It!

- Ajay Yadava


On Dec. 28, 2015, 4:05 p.m., Paul Isaychuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41742/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2015, 4:05 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1697
>     https://issues.apache.org/jira/browse/FALCON-1697
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Fixes related to stabilization of tests which are based on instance lifecycle by adding timeouts, separating data required for different instances etc, optimization scenario.
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 3d05ae9 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java 6493133 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceSuspendTest.java f673314 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java 7ad4c8e 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListProcessInstancesTest.java 43bdd87 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/EntityPageTest.java 4ad775e 
> 
> Diff: https://reviews.apache.org/r/41742/diff/
> 
> 
> Testing
> -------
> 
> Testing done.
> 
> 
> Thanks,
> 
> Paul Isaychuk
> 
>


Re: Review Request 41742: [FALCON-1697] Stabilization of scenarios which are based on instances lifecycle

Posted by Paul Isaychuk <pi...@hortonworks.com>.

> On Jan. 4, 2016, 10:40 a.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java, line 343
> > <https://reviews.apache.org/r/41742/diff/2/?file=1176901#file1176901line343>
> >
> >     Is sleep required here, I mean we have already waited for instance to reach suspended state, so why is another wait required?

While we are sure that instances were suspended, we need some more delay between suspend and resume actions.

We faced issue when instances were suspended (and waitTillInstanceReachState(...) shows the same) but because resume is performed immidiately after suspend (waitTillInstanceReachState gets suspended instances in first check iteration, so it doesn't take much time), not all instances were resumed. After adding sleep, issue didn't reproduce.


> On Jan. 4, 2016, 10:40 a.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java, line 142
> > <https://reviews.apache.org/r/41742/diff/2/?file=1176903#file1176903line142>
> >
> >     Isn't it 2nd & 3rd instances which are getting killed?

as oozie names its job instances as @1 @2 ... starting from 1, I also named the same instances as 1, 2, 3, 4 instead of 0, 1, 2, 3. Should I change a comment?


- Paul


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


On Dec. 28, 2015, 4:05 p.m., Paul Isaychuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41742/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2015, 4:05 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1697
>     https://issues.apache.org/jira/browse/FALCON-1697
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Fixes related to stabilization of tests which are based on instance lifecycle by adding timeouts, separating data required for different instances etc, optimization scenario.
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 3d05ae9 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java 6493133 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceSuspendTest.java f673314 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java 7ad4c8e 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListProcessInstancesTest.java 43bdd87 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/EntityPageTest.java 4ad775e 
> 
> Diff: https://reviews.apache.org/r/41742/diff/
> 
> 
> Testing
> -------
> 
> Testing done.
> 
> 
> Thanks,
> 
> Paul Isaychuk
> 
>


Re: Review Request 41742: [FALCON-1697] Stabilization of scenarios which are based on instances lifecycle

Posted by Ajay Yadava <aj...@gmail.com>.

> On Jan. 4, 2016, 10:40 a.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java, line 343
> > <https://reviews.apache.org/r/41742/diff/2/?file=1176901#file1176901line343>
> >
> >     Is sleep required here, I mean we have already waited for instance to reach suspended state, so why is another wait required?
> 
> Paul Isaychuk wrote:
>     While we are sure that instances were suspended, we need some more delay between suspend and resume actions.
>     
>     We faced issue when instances were suspended (and waitTillInstanceReachState(...) shows the same) but because resume is performed immidiately after suspend (waitTillInstanceReachState gets suspended instances in first check iteration, so it doesn't take much time), not all instances were resumed. After adding sleep, issue didn't reproduce.

Ok. Makes sense.


- Ajay


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


On Dec. 28, 2015, 4:05 p.m., Paul Isaychuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41742/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2015, 4:05 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1697
>     https://issues.apache.org/jira/browse/FALCON-1697
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Fixes related to stabilization of tests which are based on instance lifecycle by adding timeouts, separating data required for different instances etc, optimization scenario.
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 3d05ae9 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java 6493133 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceSuspendTest.java f673314 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java 7ad4c8e 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListProcessInstancesTest.java 43bdd87 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/EntityPageTest.java 4ad775e 
> 
> Diff: https://reviews.apache.org/r/41742/diff/
> 
> 
> Testing
> -------
> 
> Testing done.
> 
> 
> Thanks,
> 
> Paul Isaychuk
> 
>


Re: Review Request 41742: [FALCON-1697] Stabilization of scenarios which are based on instances lifecycle

Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41742/#review112203
-----------------------------------------------------------



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java (line 343)
<https://reviews.apache.org/r/41742/#comment172544>

    Is sleep required here, I mean we have already waited for instance to reach suspended state, so why is another wait required?



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java (line 136)
<https://reviews.apache.org/r/41742/#comment173031>

    Isn't it 2nd & 3rd instances which are getting killed?


- Ajay Yadava


On Dec. 28, 2015, 4:05 p.m., Paul Isaychuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41742/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2015, 4:05 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1697
>     https://issues.apache.org/jira/browse/FALCON-1697
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Fixes related to stabilization of tests which are based on instance lifecycle by adding timeouts, separating data required for different instances etc, optimization scenario.
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 3d05ae9 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java 6493133 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceSuspendTest.java f673314 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java 7ad4c8e 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListProcessInstancesTest.java 43bdd87 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/EntityPageTest.java 4ad775e 
> 
> Diff: https://reviews.apache.org/r/41742/diff/
> 
> 
> Testing
> -------
> 
> Testing done.
> 
> 
> Thanks,
> 
> Paul Isaychuk
> 
>


Re: Review Request 41742: [FALCON-1697] Stabilization of scenarios which are based on instances lifecycle

Posted by Paul Isaychuk <pi...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41742/
-----------------------------------------------------------

(Updated Dec. 28, 2015, 4:05 p.m.)


Review request for Falcon.


Bugs: FALCON-1697
    https://issues.apache.org/jira/browse/FALCON-1697


Repository: falcon-git


Description
-------

Fixes related to stabilization of tests which are based on instance lifecycle by adding timeouts, separating data required for different instances etc, optimization scenario.


Diffs (updated)
-----

  falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 3d05ae9 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceStatusTest.java 6493133 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/ProcessInstanceSuspendTest.java f673314 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java 7ad4c8e 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListProcessInstancesTest.java 43bdd87 
  falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/EntityPageTest.java 4ad775e 

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


Testing
-------

Testing done.


Thanks,

Paul Isaychuk