Hi Nikolai, will you have time to merge this change? вт, 20 мар. 2018 г. в 11:52, Dmitry Pavlov <dpavlov....@gmail.com>:
> Dmitriy, thank you for review. This fix should do our tests more stable. > > Nikolay, could you please merge? > > вт, 20 мар. 2018 г. в 11:49, Dmitriy Govorukhin < > dmitriy.govoruk...@gmail.com>: > >> Looks good for me, please merge. >> >> 19 мар. 2018 г. 3:22 ПП пользователь "Dmitry Pavlov" < >> dpavlov....@gmail.com> написал: >> >> I agree it is important, I'm going to do a review, but do not have time >>> slot to do. >>> >>> Who could pick up this review? >>> >>> Dmitriy G., could I ask you? >>> >>> пн, 19 мар. 2018 г. в 15:13, Maxim Muzafarov <maxmu...@gmail.com>: >>> >>>> Dmitry and other igniters, >>>> >>>> Will you have time to review this issue? >>>> I've preperated PR and TC for this, also I've fixed all comments made >>>> by Andrey Kuznetsov and Vyacheslav Daradur. >>>> >>>> I think this is important issue and will make test framework more >>>> stable and clear. >>>> >>>> >>>> TC: https://ci.ignite.apache.org/viewLog.html?buildId=1138151 >>>> JIRA: https://issues.apache.org/jira/browse/IGNITE-6842 >>>> Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502 >>>> PR: https://github.com/apache/ignite/pull/3542 >>>> >>>> чт, 15 мар. 2018 г. в 13:31, Maxim Muzafarov <maxmu...@gmail.com>: >>>> >>>>> Dmtry, >>>>> >>>>> Can we proceed with this change? >>>>> I've done with fixing review comments and tests that you mentioned >>>>> before. >>>>> >>>>> TC: https://ci.ignite.apache.org/viewLog.html?buildId=1138151 >>>>> JIRA: https://issues.apache.org/jira/browse/IGNITE-6842 >>>>> Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502 >>>>> PR: https://github.com/apache/ignite/pull/3542 >>>>> >>>>> >>>>> >>>>> вт, 6 мар. 2018 г. в 20:42, Dmitry Pavlov <dpavlov....@gmail.com>: >>>>> >>>>>> Ok, thank you. >>>>>> >>>>>> Please let me know when we can proceed with review >>>>>> https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502 >>>>>> >>>>>> >>>>>> вт, 6 мар. 2018 г. в 20:17, Maxim Muzafarov <maxmu...@gmail.com>: >>>>>> >>>>>> > Hello Dmitry, >>>>>> > >>>>>> > Yes, I've updated test classes as you metioned before. >>>>>> > Now i'm fixing review comments. Within next few days I'll prepare >>>>>> final >>>>>> > version of this PR. >>>>>> > >>>>>> > вт, 6 мар. 2018 г. в 20:12, Dmitry Pavlov <dpavlov....@gmail.com>: >>>>>> > >>>>>> > > Hi Maxim, >>>>>> > > >>>>>> > > are there any news on these test fails? >>>>>> > > >>>>>> > > Is issue ready for review? >>>>>> > > >>>>>> > > Sincerely, >>>>>> > > Dmitiry Pavlov >>>>>> > > >>>>>> > > вт, 27 февр. 2018 г. в 17:12, Dmitry Pavlov < >>>>>> dpavlov....@gmail.com>: >>>>>> > > >>>>>> > > > Hi, thank you! >>>>>> > > > >>>>>> > > > I've found several suspicious fails: such test fails have rate >>>>>> less >>>>>> > than >>>>>> > > > 1%, it is probably new failures. >>>>>> > > > >>>>>> > > > It would be great if we can fix it before merge. Could you >>>>>> address this >>>>>> > > > fails? >>>>>> > > > >>>>>> > > > Sincerely, >>>>>> > > > Dmitriy Pavlov >>>>>> > > > >>>>>> > > > IgniteCacheTestSuite5: >>>>>> IgniteCacheStoreCollectionTest.testStoreMap >>>>>> > (fail >>>>>> > > > rate 0,0%) >>>>>> > > > IgniteCacheTestSuite5: >>>>>> > > > CacheLateAffinityAssignmentTest.testDelayAssignmentClientJoin >>>>>> (fail >>>>>> > rate >>>>>> > > > 0,0%) >>>>>> > > > IgniteCacheWithIndexingTestSuite: >>>>>> > > > >>>>>> CacheRandomOperationsMultithreadedTest.testAtomicOffheapEviction (fail >>>>>> > > rate >>>>>> > > > 0,0%) >>>>>> > > > IgniteCacheWithIndexingTestSuite: >>>>>> > > > >>>>>> > >>>>>> CacheRandomOperationsMultithreadedTest.testAtomicOffheapEvictionIndexing >>>>>> > > > (fail rate 0,0%) >>>>>> > > > IgniteCacheWithIndexingTestSuite: >>>>>> > > > CacheRandomOperationsMultithreadedTest.testTxOffheapEviction >>>>>> (fail rate >>>>>> > > > 0,0%) >>>>>> > > > IgniteCacheWithIndexingTestSuite: >>>>>> > > > >>>>>> CacheRandomOperationsMultithreadedTest.testTxOffheapEvictionIndexing >>>>>> > > (fail >>>>>> > > > rate 0,0%) >>>>>> > > > >>>>>> > > > IgniteBinarySimpleNameMapperCacheFullApiTestSuite: >>>>>> > > > >>>>>> > > >>>>>> > >>>>>> GridCachePartitionedNearDisabledMultiNodeWithGroupFullApiSelfTest.testWithSkipStoreTx >>>>>> > > > (fail rate 0,0%) >>>>>> > > > >>>>>> > > > вт, 27 февр. 2018 г. в 17:04, Maxim Muzafarov < >>>>>> maxmu...@gmail.com>: >>>>>> > > > >>>>>> > > >> Yep, link may not be correct. >>>>>> > > >> >>>>>> > > >> Here is correct version: >>>>>> > > >> TC: * >>>>>> > > >> >>>>>> > > >>>>>> > >>>>>> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&branch_IgniteTests24Java8=pull%2F3542%2Fhead >>>>>> > > >> < >>>>>> > > >> >>>>>> > > >>>>>> > >>>>>> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&branch_IgniteTests24Java8=pull%2F3542%2Fhead >>>>>> > > >> >* >>>>>> > > >> >>>>>> > > >> >>>>>> > > >> вт, 27 февр. 2018 г. в 16:41, Dmitry Pavlov < >>>>>> dpavlov....@gmail.com>: >>>>>> > > >> >>>>>> > > >> > Hi Maxim, >>>>>> > > >> > >>>>>> > > >> > could you please provide link to TC run on your PR? It seems >>>>>> link >>>>>> > > >> provided >>>>>> > > >> > points to run of master. In changes field you may select >>>>>> > > pull/3542/head >>>>>> > > >> > before starting RunAll. >>>>>> > > >> > >>>>>> > > >> > Igniters, >>>>>> > > >> > >>>>>> > > >> > this change is related to our test framework, so change may >>>>>> affect >>>>>> > > your >>>>>> > > >> > tests. Please join to review >>>>>> > > >> > https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502 >>>>>> > > >> > >>>>>> > > >> > Sincerely, >>>>>> > > >> > Dmitriy Pavlov >>>>>> > > >> > >>>>>> > > >> > вт, 27 февр. 2018 г. в 16:14, Maxim Muzafarov < >>>>>> maxmu...@gmail.com>: >>>>>> > > >> > >>>>>> > > >> > > Hi all, >>>>>> > > >> > > >>>>>> > > >> > > I think, I've done with this issue, what should we do next? >>>>>> > > >> > > >>>>>> > > >> > > PR: https://github.com/apache/ignite/pull/3542 >>>>>> > > >> > > Upsource: >>>>>> > > https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502 >>>>>> > > >> > > TC: >>>>>> > > >> > > >>>>>> > > >> > > >>>>>> > > >> > >>>>>> > > >> >>>>>> > > >>>>>> > >>>>>> https://ci.ignite.apache.org/viewModification.html?modId=723895&personal=false&buildTypeId=&tab=vcsModificationTests >>>>>> > > >> > > JIRA: https://issues.apache.org/jira/browse/IGNITE-6842 >>>>>> > > >> > > >>>>>> > > >> > > >>>>>> > > >> > > чт, 22 февр. 2018 г. в 14:12, Dmitry Pavlov < >>>>>> > dpavlov....@gmail.com >>>>>> > > >: >>>>>> > > >> > > >>>>>> > > >> > > > Hi Maxim, >>>>>> > > >> > > > >>>>>> > > >> > > > Thank you. >>>>>> > > >> > > > >>>>>> > > >> > > > To my mind stopAllGrids call should be kept in >>>>>> afterTestsStop(). >>>>>> > > If >>>>>> > > >> > > > developer forgot to call super(), he will see exception >>>>>> from >>>>>> > > >> > > > beforeTestsStart() >>>>>> > > >> > > > added by you. >>>>>> > > >> > > > >>>>>> > > >> > > > If you think patch is ready to be reviewed, please >>>>>> change JIRA >>>>>> > > >> status >>>>>> > > >> > to >>>>>> > > >> > > > "Patch Available". >>>>>> > > >> > > > >>>>>> > > >> > > > Optionally you can create Upsource review. It would be >>>>>> easier >>>>>> > for >>>>>> > > >> > > multiple >>>>>> > > >> > > > reviewers to handle this patch. This test framework is >>>>>> used by >>>>>> > all >>>>>> > > >> > > Igniters >>>>>> > > >> > > > so Upsource would be good option ( >>>>>> > > >> https://reviews.ignite.apache.org/ >>>>>> > > >> > ). >>>>>> > > >> > > > >>>>>> > > >> > > > Sincerely, >>>>>> > > >> > > > Dmitriy Pavlov >>>>>> > > >> > > > >>>>>> > > >> > > > чт, 22 февр. 2018 г. в 13:44, Maxim Muzafarov < >>>>>> > maxmu...@gmail.com >>>>>> > > >: >>>>>> > > >> > > > >>>>>> > > >> > > > > Hi all, >>>>>> > > >> > > > > >>>>>> > > >> > > > > I've made some changes based on our previous >>>>>> discusstions, >>>>>> > > please >>>>>> > > >> see >>>>>> > > >> > > PR >>>>>> > > >> > > > > [1]: >>>>>> > > >> > > > > 1) Remove duplicated code for stopGrid (by index and >>>>>> by name); >>>>>> > > >> > > > > 2) Add new method that thows exception if not all grids >>>>>> > haven't >>>>>> > > >> > stopped >>>>>> > > >> > > > > correctly; >>>>>> > > >> > > > > 3) Change tests that have been affected by this >>>>>> changes; >>>>>> > > >> > > > > >>>>>> > > >> > > > > Also, I have some thoughts for clarification: >>>>>> > > >> > > > > 1) beforeTestsStart() - I expect here in subtests that >>>>>> grids >>>>>> > are >>>>>> > > >> not >>>>>> > > >> > > > > started yet. Am I right? >>>>>> > > >> > > > > 2) I think we should call stopAllGrids in tearDown >>>>>> method. So, >>>>>> > > if >>>>>> > > >> in >>>>>> > > >> > > some >>>>>> > > >> > > > > cases we'll override afterTestsStop in subclasses and >>>>>> forgot >>>>>> > to >>>>>> > > >> call >>>>>> > > >> > > > > super() it won't lead us to exception. >>>>>> > > >> > > > > >>>>>> > > >> > > > > [1] https://github.com/apache/ignite/pull/3542 >>>>>> > > >> > > > > [2] >>>>>> > > >> >>>>>> https://ci.ignite.apache.org/viewModification.html?modId=717275 >>>>>> > > >> > > > > [3] https://issues.apache.org/jira/browse/IGNITE-6842 >>>>>> > > >> > > > > >>>>>> > > >> > > > > >>>>>> > > >> > > > > ср, 7 февр. 2018 г. в 18:28, Maxim Muzafarov < >>>>>> > > maxmu...@gmail.com >>>>>> > > >> >: >>>>>> > > >> > > > > >>>>>> > > >> > > > > > Thank you all, >>>>>> > > >> > > > > > >>>>>> > > >> > > > > > I'll add this comment's for JIRA ticket, if you >>>>>> don't mind. >>>>>> > > >> > > > > > >>>>>> > > >> > > > > > ср, 7 февр. 2018 г. в 15:16, Dmitry Pavlov < >>>>>> > > >> dpavlov....@gmail.com >>>>>> > > >> > >: >>>>>> > > >> > > > > > >>>>>> > > >> > > > > >> Yes, this solution allows to cover both cases: >>>>>> > > >> > > > > >> a) not stopped node from previous test and >>>>>> > > >> > > > > >> b) allows to remove useless code that stops Ignite >>>>>> nodes >>>>>> > from >>>>>> > > >> each >>>>>> > > >> > > > test. >>>>>> > > >> > > > > >> >>>>>> > > >> > > > > >> ср, 7 февр. 2018 г. в 15:13, Anton Vinogradov < >>>>>> > > >> > > > avinogra...@gridgain.com >>>>>> > > >> > > > > >: >>>>>> > > >> > > > > >> >>>>>> > > >> > > > > >> > Maxim, >>>>>> > > >> > > > > >> > >>>>>> > > >> > > > > >> > We discussed with Dima privately, and decided >>>>>> > > >> > > > > >> > >>>>>> > > >> > > > > >> > 1) We have to assert that there is no alive nodes >>>>>> at >>>>>> > > >> > > > > GridAbstractTest's >>>>>> > > >> > > > > >> > beforeTestsStarted >>>>>> > > >> > > > > >> > 2) We have to kill all alive nodes (without >>>>>> force) at >>>>>> > > >> > > > > GridAbstractTest's >>>>>> > > >> > > > > >> > afterTestsStopped >>>>>> > > >> > > > > >> > 3) In case of any exceptions at #2 we have to see >>>>>> test >>>>>> > > error >>>>>> > > >> > > > > >> > 4) We can get rid of all useless stopAllGrids at >>>>>> > > >> > > GridAbstractTest's >>>>>> > > >> > > > > >> > subclasses. >>>>>> > > >> > > > > >> > >>>>>> > > >> > > > > >> > >>>>>> > > >> > > > > >> > >>>>>> > > >> > > > > >> > On Wed, Feb 7, 2018 at 2:32 PM, Dmitry Pavlov < >>>>>> > > >> > > > dpavlov....@gmail.com> >>>>>> > > >> > > > > >> > wrote: >>>>>> > > >> > > > > >> > >>>>>> > > >> > > > > >> > > > Let's just add stopAllGrids(flase) to >>>>>> > GridAbstractTest >>>>>> > > 's >>>>>> > > >> > > > > >> > > afterTestsStopped >>>>>> > > >> > > > > >> > > method body. >>>>>> > > >> > > > > >> > > >>>>>> > > >> > > > > >> > > Can't agree with it becase implicit silent >>>>>> shutdown of >>>>>> > > >> nodes >>>>>> > > >> > > from >>>>>> > > >> > > > > test >>>>>> > > >> > > > > >> > > framework may cause a lot of bugs introduced to >>>>>> Ignite. >>>>>> > > >> > > > > >> > > >>>>>> > > >> > > > > >> > > I think stop+test fail should occur. >>>>>> > > >> > > > > >> > > In that case author of incorrect test or not >>>>>> consistent >>>>>> > > >> Ignite >>>>>> > > >> > > > > >> shutdown >>>>>> > > >> > > > > >> > > will see problem. >>>>>> > > >> > > > > >> > > >>>>>> > > >> > > > > >> > > 'Fail fast' if always better than hidding real >>>>>> problem >>>>>> > > >> under >>>>>> > > >> > > > > automatic >>>>>> > > >> > > > > >> > > framework feature. >>>>>> > > >> > > > > >> > > >>>>>> > > >> > > > > >> > > ср, 7 февр. 2018 г. в 14:05, Anton Vinogradov < >>>>>> > > >> > > > > >> avinogra...@gridgain.com >>>>>> > > >> > > > > >> > >: >>>>>> > > >> > > > > >> > > >>>>>> > > >> > > > > >> > > > Hi all, >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > > > I've made some research about this problem >>>>>> and i >>>>>> > > think >>>>>> > > >> > that >>>>>> > > >> > > in >>>>>> > > >> > > > > >> > general >>>>>> > > >> > > > > >> > > we >>>>>> > > >> > > > > >> > > > > should move stopAllGrids method in >>>>>> GridAbstractTest >>>>>> > > >> class >>>>>> > > >> > to >>>>>> > > >> > > > > >> > > > > afterTestsStopped method with some changes. >>>>>> Am I >>>>>> > > right? >>>>>> > > >> > > > > >> > > > Let's just add stopAllGrids(flase) to >>>>>> > GridAbstractTest >>>>>> > > 's >>>>>> > > >> > > > > >> > > > afterTestsStopped method >>>>>> > > >> > > > > >> > > > body. >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > > > I have a question about stopAllGrids(boolean >>>>>> > cancel) >>>>>> > > >> this >>>>>> > > >> > > > > "cancel" >>>>>> > > >> > > > > >> > > > That's just a flag means "do not wait for any >>>>>> > > operations >>>>>> > > >> > > finish" >>>>>> > > >> > > > > >> > > > See no reason to set it true in that case. >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > > > If you delegate closing to >>>>>> afterTestsStopped this >>>>>> > > will >>>>>> > > >> > > affect >>>>>> > > >> > > > > only >>>>>> > > >> > > > > >> > > > > last test (method). >>>>>> > > >> > > > > >> > > > The idea is to stop all nodes at last test's >>>>>> method >>>>>> > > >> finish. >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > > > Nodes that survive between tests can affect >>>>>> > > successive >>>>>> > > >> > > > > >> > > > tests. >>>>>> > > >> > > > > >> > > > Thats ok. we have a lot tests where nodes >>>>>> reusable >>>>>> > > >> between >>>>>> > > >> > > > test's >>>>>> > > >> > > > > >> > > methods. >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > > On Wed, Feb 7, 2018 at 1:20 PM, Dmitry Pavlov >>>>>> < >>>>>> > > >> > > > > >> dpavlov....@gmail.com> >>>>>> > > >> > > > > >> > > > wrote: >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > > > Hi Igniters, >>>>>> > > >> > > > > >> > > > > >>>>>> > > >> > > > > >> > > > > IMO nodes that survive between tests is not >>>>>> general >>>>>> > > >> > practice >>>>>> > > >> > > > and >>>>>> > > >> > > > > >> I'm >>>>>> > > >> > > > > >> > > not >>>>>> > > >> > > > > >> > > > > sure is a best practice. I suggest to mark >>>>>> such >>>>>> > tests >>>>>> > > >> with >>>>>> > > >> > > > some >>>>>> > > >> > > > > >> > method >>>>>> > > >> > > > > >> > > > > overriden from AbstractTest. >>>>>> > > >> > > > > >> > > > > >>>>>> > > >> > > > > >> > > > > About cancel flag, please note >>>>>> stopAllGrids(boolean >>>>>> > > >> > cancel) >>>>>> > > >> > > > > >> > > cancel=false >>>>>> > > >> > > > > >> > > > > allows to wait of checkpoint ends in case >>>>>> > persistence >>>>>> > > >> > > enabled. >>>>>> > > >> > > > > >> > > > > >>>>>> > > >> > > > > >> > > > > I still suggest to avoid stopping any nodes >>>>>> by >>>>>> > test, >>>>>> > > >> but >>>>>> > > >> > > > > validate >>>>>> > > >> > > > > >> not >>>>>> > > >> > > > > >> > > > > stopped node exist and fail test instead of >>>>>> siltent >>>>>> > > >> > implicit >>>>>> > > >> > > > > >> actions. >>>>>> > > >> > > > > >> > > > > >>>>>> > > >> > > > > >> > > > > Sincerely, >>>>>> > > >> > > > > >> > > > > Dmitriy Pavlov >>>>>> > > >> > > > > >> > > > > >>>>>> > > >> > > > > >> > > > > ср, 7 февр. 2018 г. в 13:04, Andrey >>>>>> Kuznetsov < >>>>>> > > >> > > > > stku...@gmail.com >>>>>> > > >> > > > > >> >: >>>>>> > > >> > > > > >> > > > > >>>>>> > > >> > > > > >> > > > > > Hi Maxim, >>>>>> > > >> > > > > >> > > > > > >>>>>> > > >> > > > > >> > > > > > Regarding your first question, the use of >>>>>> > > >> > > afterTestsStopped >>>>>> > > >> > > > is >>>>>> > > >> > > > > >> not >>>>>> > > >> > > > > >> > > > enough >>>>>> > > >> > > > > >> > > > > > to stop all nodes, since each individual >>>>>> test >>>>>> > > >> (method) >>>>>> > > >> > can >>>>>> > > >> > > > > start >>>>>> > > >> > > > > >> > > custom >>>>>> > > >> > > > > >> > > > > set >>>>>> > > >> > > > > >> > > > > > of notes during its operation, and this >>>>>> very test >>>>>> > > >> should >>>>>> > > >> > > > stop >>>>>> > > >> > > > > >> all >>>>>> > > >> > > > > >> > > those >>>>>> > > >> > > > > >> > > > > > nodes. If you delegate closing to >>>>>> > afterTestsStopped >>>>>> > > >> this >>>>>> > > >> > > > will >>>>>> > > >> > > > > >> > affect >>>>>> > > >> > > > > >> > > > only >>>>>> > > >> > > > > >> > > > > > last test (method). Nodes that survive >>>>>> between >>>>>> > > tests >>>>>> > > >> can >>>>>> > > >> > > > > affect >>>>>> > > >> > > > > >> > > > > successive >>>>>> > > >> > > > > >> > > > > > tests. >>>>>> > > >> > > > > >> > > > > > >>>>>> > > >> > > > > >> > > > > > 2018-02-07 1:10 GMT+03:00 Maxim Muzafarov >>>>>> < >>>>>> > > >> > > > maxmu...@gmail.com >>>>>> > > >> > > > > >: >>>>>> > > >> > > > > >> > > > > > >>>>>> > > >> > > > > >> > > > > > > Hello, >>>>>> > > >> > > > > >> > > > > > > >>>>>> > > >> > > > > >> > > > > > > I've made some research about this >>>>>> problem and >>>>>> > i >>>>>> > > >> think >>>>>> > > >> > > > that >>>>>> > > >> > > > > in >>>>>> > > >> > > > > >> > > > general >>>>>> > > >> > > > > >> > > > > we >>>>>> > > >> > > > > >> > > > > > > should move stopAllGrids method in >>>>>> > > GridAbstractTest >>>>>> > > >> > > class >>>>>> > > >> > > > to >>>>>> > > >> > > > > >> > > > > > > afterTestsStopped method with some >>>>>> changes. Am >>>>>> > I >>>>>> > > >> > right? >>>>>> > > >> > > > > >> > > > > > > >>>>>> > > >> > > > > >> > > > > > > Also, I have a question about >>>>>> > > stopAllGrids(boolean >>>>>> > > >> > > cancel) >>>>>> > > >> > > > > >> this >>>>>> > > >> > > > > >> > > > > "cancel" >>>>>> > > >> > > > > >> > > > > > > argument. Why in some cases we should >>>>>> interrupt >>>>>> > > >> > > ComputeJob >>>>>> > > >> > > > > >> and in >>>>>> > > >> > > > > >> > > > some >>>>>> > > >> > > > > >> > > > > > > cases shouldn't? For example here: >>>>>> > > >> > > > > >> > > > > > > >>>>>> > > >> IgniteBaselineAffinityTopologyActivationTest#afterTest >>>>>> > > >> > > > > >> > > > > > > we call method stopAllGrids(false) this >>>>>> way. >>>>>> > Why >>>>>> > > >> not >>>>>> > > >> > > > "true" >>>>>> > > >> > > > > >> > > argument >>>>>> > > >> > > > > >> > > > > > > instead? >>>>>> > > >> > > > > >> > > > > > > >>>>>> > > >> > > > > >> > > > > > > >>>>>> > > >> > > > > >> > > > > > > -- >>>>>> > > >> > > > > >> > > > > > Best regards, >>>>>> > > >> > > > > >> > > > > > Andrey Kuznetsov. >>>>>> > > >> > > > > >> > > > > > >>>>>> > > >> > > > > >> > > > > >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > >>>>>> > > >> > > > > >> > >>>>>> > > >> > > > > >> >>>>>> > > >> > > > > > >>>>>> > > >> > > > > >>>>>> > > >> > > > >>>>>> > > >> > > >>>>>> > > >> > >>>>>> > > >> >>>>>> > > > >>>>>> > > >>>>>> > >>>>> >>>>> ikolai, would you have time to merge this change? вт, 20 мар. 2018 г. в 11:52, Dmitry Pavlov <dpavlov....@gmail.com>: > Dmitriy, thank you for review. This fix should do our tests more stable. > > Nikolay, could you please merge? > > вт, 20 мар. 2018 г. в 11:49, Dmitriy Govorukhin < > dmitriy.govoruk...@gmail.com>: > >> Looks good for me, please merge. >> >> 19 мар. 2018 г. 3:22 ПП пользователь "Dmitry Pavlov" < >> dpavlov....@gmail.com> написал: >> >> I agree it is important, I'm going to do a review, but do not have time >>> slot to do. >>> >>> Who could pick up this review? >>> >>> Dmitriy G., could I ask you? >>> >>> пн, 19 мар. 2018 г. в 15:13, Maxim Muzafarov <maxmu...@gmail.com>: >>> >>>> Dmitry and other igniters, >>>> >>>> Will you have time to review this issue? >>>> I've preperated PR and TC for this, also I've fixed all comments made >>>> by Andrey Kuznetsov and Vyacheslav Daradur. >>>> >>>> I think this is important issue and will make test framework more >>>> stable and clear. >>>> >>>> >>>> TC: https://ci.ignite.apache.org/viewLog.html?buildId=1138151 >>>> JIRA: https://issues.apache.org/jira/browse/IGNITE-6842 >>>> Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502 >>>> PR: https://github.com/apache/ignite/pull/3542 >>>> >>>> чт, 15 мар. 2018 г. в 13:31, Maxim Muzafarov <maxmu...@gmail.com>: >>>> >>>>> Dmtry, >>>>> >>>>> Can we proceed with this change? >>>>> I've done with fixing review comments and tests that you mentioned >>>>> before. >>>>> >>>>> TC: https://ci.ignite.apache.org/viewLog.html?buildId=1138151 >>>>> JIRA: https://issues.apache.org/jira/browse/IGNITE-6842 >>>>> Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502 >>>>> PR: https://github.com/apache/ignite/pull/3542 >>>>> >>>>> >>>>> >>>>> вт, 6 мар. 2018 г. в 20:42, Dmitry Pavlov <dpavlov....@gmail.com>: >>>>> >>>>>> Ok, thank you. >>>>>> >>>>>> Please let me know when we can proceed with review >>>>>> https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502 >>>>>> >>>>>> >>>>>> вт, 6 мар. 2018 г. в 20:17, Maxim Muzafarov <maxmu...@gmail.com>: >>>>>> >>>>>> > Hello Dmitry, >>>>>> > >>>>>> > Yes, I've updated test classes as you metioned before. >>>>>> > Now i'm fixing review comments. Within next few days I'll prepare >>>>>> final >>>>>> > version of this PR. >>>>>> > >>>>>> > вт, 6 мар. 2018 г. в 20:12, Dmitry Pavlov <dpavlov....@gmail.com>: >>>>>> > >>>>>> > > Hi Maxim, >>>>>> > > >>>>>> > > are there any news on these test fails? >>>>>> > > >>>>>> > > Is issue ready for review? >>>>>> > > >>>>>> > > Sincerely, >>>>>> > > Dmitiry Pavlov >>>>>> > > >>>>>> > > вт, 27 февр. 2018 г. в 17:12, Dmitry Pavlov < >>>>>> dpavlov....@gmail.com>: >>>>>> > > >>>>>> > > > Hi, thank you! >>>>>> > > > >>>>>> > > > I've found several suspicious fails: such test fails have rate >>>>>> less >>>>>> > than >>>>>> > > > 1%, it is probably new failures. >>>>>> > > > >>>>>> > > > It would be great if we can fix it before merge. Could you >>>>>> address this >>>>>> > > > fails? >>>>>> > > > >>>>>> > > > Sincerely, >>>>>> > > > Dmitriy Pavlov >>>>>> > > > >>>>>> > > > IgniteCacheTestSuite5: >>>>>> IgniteCacheStoreCollectionTest.testStoreMap >>>>>> > (fail >>>>>> > > > rate 0,0%) >>>>>> > > > IgniteCacheTestSuite5: >>>>>> > > > CacheLateAffinityAssignmentTest.testDelayAssignmentClientJoin >>>>>> (fail >>>>>> > rate >>>>>> > > > 0,0%) >>>>>> > > > IgniteCacheWithIndexingTestSuite: >>>>>> > > > >>>>>> CacheRandomOperationsMultithreadedTest.testAtomicOffheapEviction (fail >>>>>> > > rate >>>>>> > > > 0,0%) >>>>>> > > > IgniteCacheWithIndexingTestSuite: >>>>>> > > > >>>>>> > >>>>>> CacheRandomOperationsMultithreadedTest.testAtomicOffheapEvictionIndexing >>>>>> > > > (fail rate 0,0%) >>>>>> > > > IgniteCacheWithIndexingTestSuite: >>>>>> > > > CacheRandomOperationsMultithreadedTest.testTxOffheapEviction >>>>>> (fail rate >>>>>> > > > 0,0%) >>>>>> > > > IgniteCacheWithIndexingTestSuite: >>>>>> > > > >>>>>> CacheRandomOperationsMultithreadedTest.testTxOffheapEvictionIndexing >>>>>> > > (fail >>>>>> > > > rate 0,0%) >>>>>> > > > >>>>>> > > > IgniteBinarySimpleNameMapperCacheFullApiTestSuite: >>>>>> > > > >>>>>> > > >>>>>> > >>>>>> GridCachePartitionedNearDisabledMultiNodeWithGroupFullApiSelfTest.testWithSkipStoreTx >>>>>> > > > (fail rate 0,0%) >>>>>> > > > >>>>>> > > > вт, 27 февр. 2018 г. в 17:04, Maxim Muzafarov < >>>>>> maxmu...@gmail.com>: >>>>>> > > > >>>>>> > > >> Yep, link may not be correct. >>>>>> > > >> >>>>>> > > >> Here is correct version: >>>>>> > > >> TC: * >>>>>> > > >> >>>>>> > > >>>>>> > >>>>>> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&branch_IgniteTests24Java8=pull%2F3542%2Fhead >>>>>> > > >> < >>>>>> > > >> >>>>>> > > >>>>>> > >>>>>> https://ci.ignite.apache.org/project.html?projectId=IgniteTests24Java8&branch_IgniteTests24Java8=pull%2F3542%2Fhead >>>>>> > > >> >* >>>>>> > > >> >>>>>> > > >> >>>>>> > > >> вт, 27 февр. 2018 г. в 16:41, Dmitry Pavlov < >>>>>> dpavlov....@gmail.com>: >>>>>> > > >> >>>>>> > > >> > Hi Maxim, >>>>>> > > >> > >>>>>> > > >> > could you please provide link to TC run on your PR? It seems >>>>>> link >>>>>> > > >> provided >>>>>> > > >> > points to run of master. In changes field you may select >>>>>> > > pull/3542/head >>>>>> > > >> > before starting RunAll. >>>>>> > > >> > >>>>>> > > >> > Igniters, >>>>>> > > >> > >>>>>> > > >> > this change is related to our test framework, so change may >>>>>> affect >>>>>> > > your >>>>>> > > >> > tests. Please join to review >>>>>> > > >> > https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502 >>>>>> > > >> > >>>>>> > > >> > Sincerely, >>>>>> > > >> > Dmitriy Pavlov >>>>>> > > >> > >>>>>> > > >> > вт, 27 февр. 2018 г. в 16:14, Maxim Muzafarov < >>>>>> maxmu...@gmail.com>: >>>>>> > > >> > >>>>>> > > >> > > Hi all, >>>>>> > > >> > > >>>>>> > > >> > > I think, I've done with this issue, what should we do next? >>>>>> > > >> > > >>>>>> > > >> > > PR: https://github.com/apache/ignite/pull/3542 >>>>>> > > >> > > Upsource: >>>>>> > > https://reviews.ignite.apache.org/ignite/review/IGNT-CR-502 >>>>>> > > >> > > TC: >>>>>> > > >> > > >>>>>> > > >> > > >>>>>> > > >> > >>>>>> > > >> >>>>>> > > >>>>>> > >>>>>> https://ci.ignite.apache.org/viewModification.html?modId=723895&personal=false&buildTypeId=&tab=vcsModificationTests >>>>>> > > >> > > JIRA: https://issues.apache.org/jira/browse/IGNITE-6842 >>>>>> > > >> > > >>>>>> > > >> > > >>>>>> > > >> > > чт, 22 февр. 2018 г. в 14:12, Dmitry Pavlov < >>>>>> > dpavlov....@gmail.com >>>>>> > > >: >>>>>> > > >> > > >>>>>> > > >> > > > Hi Maxim, >>>>>> > > >> > > > >>>>>> > > >> > > > Thank you. >>>>>> > > >> > > > >>>>>> > > >> > > > To my mind stopAllGrids call should be kept in >>>>>> afterTestsStop(). >>>>>> > > If >>>>>> > > >> > > > developer forgot to call super(), he will see exception >>>>>> from >>>>>> > > >> > > > beforeTestsStart() >>>>>> > > >> > > > added by you. >>>>>> > > >> > > > >>>>>> > > >> > > > If you think patch is ready to be reviewed, please >>>>>> change JIRA >>>>>> > > >> status >>>>>> > > >> > to >>>>>> > > >> > > > "Patch Available". >>>>>> > > >> > > > >>>>>> > > >> > > > Optionally you can create Upsource review. It would be >>>>>> easier >>>>>> > for >>>>>> > > >> > > multiple >>>>>> > > >> > > > reviewers to handle this patch. This test framework is >>>>>> used by >>>>>> > all >>>>>> > > >> > > Igniters >>>>>> > > >> > > > so Upsource would be good option ( >>>>>> > > >> https://reviews.ignite.apache.org/ >>>>>> > > >> > ). >>>>>> > > >> > > > >>>>>> > > >> > > > Sincerely, >>>>>> > > >> > > > Dmitriy Pavlov >>>>>> > > >> > > > >>>>>> > > >> > > > чт, 22 февр. 2018 г. в 13:44, Maxim Muzafarov < >>>>>> > maxmu...@gmail.com >>>>>> > > >: >>>>>> > > >> > > > >>>>>> > > >> > > > > Hi all, >>>>>> > > >> > > > > >>>>>> > > >> > > > > I've made some changes based on our previous >>>>>> discusstions, >>>>>> > > please >>>>>> > > >> see >>>>>> > > >> > > PR >>>>>> > > >> > > > > [1]: >>>>>> > > >> > > > > 1) Remove duplicated code for stopGrid (by index and >>>>>> by name); >>>>>> > > >> > > > > 2) Add new method that thows exception if not all grids >>>>>> > haven't >>>>>> > > >> > stopped >>>>>> > > >> > > > > correctly; >>>>>> > > >> > > > > 3) Change tests that have been affected by this >>>>>> changes; >>>>>> > > >> > > > > >>>>>> > > >> > > > > Also, I have some thoughts for clarification: >>>>>> > > >> > > > > 1) beforeTestsStart() - I expect here in subtests that >>>>>> grids >>>>>> > are >>>>>> > > >> not >>>>>> > > >> > > > > started yet. Am I right? >>>>>> > > >> > > > > 2) I think we should call stopAllGrids in tearDown >>>>>> method. So, >>>>>> > > if >>>>>> > > >> in >>>>>> > > >> > > some >>>>>> > > >> > > > > cases we'll override afterTestsStop in subclasses and >>>>>> forgot >>>>>> > to >>>>>> > > >> call >>>>>> > > >> > > > > super() it won't lead us to exception. >>>>>> > > >> > > > > >>>>>> > > >> > > > > [1] https://github.com/apache/ignite/pull/3542 >>>>>> > > >> > > > > [2] >>>>>> > > >> >>>>>> https://ci.ignite.apache.org/viewModification.html?modId=717275 >>>>>> > > >> > > > > [3] https://issues.apache.org/jira/browse/IGNITE-6842 >>>>>> > > >> > > > > >>>>>> > > >> > > > > >>>>>> > > >> > > > > ср, 7 февр. 2018 г. в 18:28, Maxim Muzafarov < >>>>>> > > maxmu...@gmail.com >>>>>> > > >> >: >>>>>> > > >> > > > > >>>>>> > > >> > > > > > Thank you all, >>>>>> > > >> > > > > > >>>>>> > > >> > > > > > I'll add this comment's for JIRA ticket, if you >>>>>> don't mind. >>>>>> > > >> > > > > > >>>>>> > > >> > > > > > ср, 7 февр. 2018 г. в 15:16, Dmitry Pavlov < >>>>>> > > >> dpavlov....@gmail.com >>>>>> > > >> > >: >>>>>> > > >> > > > > > >>>>>> > > >> > > > > >> Yes, this solution allows to cover both cases: >>>>>> > > >> > > > > >> a) not stopped node from previous test and >>>>>> > > >> > > > > >> b) allows to remove useless code that stops Ignite >>>>>> nodes >>>>>> > from >>>>>> > > >> each >>>>>> > > >> > > > test. >>>>>> > > >> > > > > >> >>>>>> > > >> > > > > >> ср, 7 февр. 2018 г. в 15:13, Anton Vinogradov < >>>>>> > > >> > > > avinogra...@gridgain.com >>>>>> > > >> > > > > >: >>>>>> > > >> > > > > >> >>>>>> > > >> > > > > >> > Maxim, >>>>>> > > >> > > > > >> > >>>>>> > > >> > > > > >> > We discussed with Dima privately, and decided >>>>>> > > >> > > > > >> > >>>>>> > > >> > > > > >> > 1) We have to assert that there is no alive nodes >>>>>> at >>>>>> > > >> > > > > GridAbstractTest's >>>>>> > > >> > > > > >> > beforeTestsStarted >>>>>> > > >> > > > > >> > 2) We have to kill all alive nodes (without >>>>>> force) at >>>>>> > > >> > > > > GridAbstractTest's >>>>>> > > >> > > > > >> > afterTestsStopped >>>>>> > > >> > > > > >> > 3) In case of any exceptions at #2 we have to see >>>>>> test >>>>>> > > error >>>>>> > > >> > > > > >> > 4) We can get rid of all useless stopAllGrids at >>>>>> > > >> > > GridAbstractTest's >>>>>> > > >> > > > > >> > subclasses. >>>>>> > > >> > > > > >> > >>>>>> > > >> > > > > >> > >>>>>> > > >> > > > > >> > >>>>>> > > >> > > > > >> > On Wed, Feb 7, 2018 at 2:32 PM, Dmitry Pavlov < >>>>>> > > >> > > > dpavlov....@gmail.com> >>>>>> > > >> > > > > >> > wrote: >>>>>> > > >> > > > > >> > >>>>>> > > >> > > > > >> > > > Let's just add stopAllGrids(flase) to >>>>>> > GridAbstractTest >>>>>> > > 's >>>>>> > > >> > > > > >> > > afterTestsStopped >>>>>> > > >> > > > > >> > > method body. >>>>>> > > >> > > > > >> > > >>>>>> > > >> > > > > >> > > Can't agree with it becase implicit silent >>>>>> shutdown of >>>>>> > > >> nodes >>>>>> > > >> > > from >>>>>> > > >> > > > > test >>>>>> > > >> > > > > >> > > framework may cause a lot of bugs introduced to >>>>>> Ignite. >>>>>> > > >> > > > > >> > > >>>>>> > > >> > > > > >> > > I think stop+test fail should occur. >>>>>> > > >> > > > > >> > > In that case author of incorrect test or not >>>>>> consistent >>>>>> > > >> Ignite >>>>>> > > >> > > > > >> shutdown >>>>>> > > >> > > > > >> > > will see problem. >>>>>> > > >> > > > > >> > > >>>>>> > > >> > > > > >> > > 'Fail fast' if always better than hidding real >>>>>> problem >>>>>> > > >> under >>>>>> > > >> > > > > automatic >>>>>> > > >> > > > > >> > > framework feature. >>>>>> > > >> > > > > >> > > >>>>>> > > >> > > > > >> > > ср, 7 февр. 2018 г. в 14:05, Anton Vinogradov < >>>>>> > > >> > > > > >> avinogra...@gridgain.com >>>>>> > > >> > > > > >> > >: >>>>>> > > >> > > > > >> > > >>>>>> > > >> > > > > >> > > > Hi all, >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > > > I've made some research about this problem >>>>>> and i >>>>>> > > think >>>>>> > > >> > that >>>>>> > > >> > > in >>>>>> > > >> > > > > >> > general >>>>>> > > >> > > > > >> > > we >>>>>> > > >> > > > > >> > > > > should move stopAllGrids method in >>>>>> GridAbstractTest >>>>>> > > >> class >>>>>> > > >> > to >>>>>> > > >> > > > > >> > > > > afterTestsStopped method with some changes. >>>>>> Am I >>>>>> > > right? >>>>>> > > >> > > > > >> > > > Let's just add stopAllGrids(flase) to >>>>>> > GridAbstractTest >>>>>> > > 's >>>>>> > > >> > > > > >> > > > afterTestsStopped method >>>>>> > > >> > > > > >> > > > body. >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > > > I have a question about stopAllGrids(boolean >>>>>> > cancel) >>>>>> > > >> this >>>>>> > > >> > > > > "cancel" >>>>>> > > >> > > > > >> > > > That's just a flag means "do not wait for any >>>>>> > > operations >>>>>> > > >> > > finish" >>>>>> > > >> > > > > >> > > > See no reason to set it true in that case. >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > > > If you delegate closing to >>>>>> afterTestsStopped this >>>>>> > > will >>>>>> > > >> > > affect >>>>>> > > >> > > > > only >>>>>> > > >> > > > > >> > > > > last test (method). >>>>>> > > >> > > > > >> > > > The idea is to stop all nodes at last test's >>>>>> method >>>>>> > > >> finish. >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > > > Nodes that survive between tests can affect >>>>>> > > successive >>>>>> > > >> > > > > >> > > > tests. >>>>>> > > >> > > > > >> > > > Thats ok. we have a lot tests where nodes >>>>>> reusable >>>>>> > > >> between >>>>>> > > >> > > > test's >>>>>> > > >> > > > > >> > > methods. >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > > On Wed, Feb 7, 2018 at 1:20 PM, Dmitry Pavlov >>>>>> < >>>>>> > > >> > > > > >> dpavlov....@gmail.com> >>>>>> > > >> > > > > >> > > > wrote: >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > > > Hi Igniters, >>>>>> > > >> > > > > >> > > > > >>>>>> > > >> > > > > >> > > > > IMO nodes that survive between tests is not >>>>>> general >>>>>> > > >> > practice >>>>>> > > >> > > > and >>>>>> > > >> > > > > >> I'm >>>>>> > > >> > > > > >> > > not >>>>>> > > >> > > > > >> > > > > sure is a best practice. I suggest to mark >>>>>> such >>>>>> > tests >>>>>> > > >> with >>>>>> > > >> > > > some >>>>>> > > >> > > > > >> > method >>>>>> > > >> > > > > >> > > > > overriden from AbstractTest. >>>>>> > > >> > > > > >> > > > > >>>>>> > > >> > > > > >> > > > > About cancel flag, please note >>>>>> stopAllGrids(boolean >>>>>> > > >> > cancel) >>>>>> > > >> > > > > >> > > cancel=false >>>>>> > > >> > > > > >> > > > > allows to wait of checkpoint ends in case >>>>>> > persistence >>>>>> > > >> > > enabled. >>>>>> > > >> > > > > >> > > > > >>>>>> > > >> > > > > >> > > > > I still suggest to avoid stopping any nodes >>>>>> by >>>>>> > test, >>>>>> > > >> but >>>>>> > > >> > > > > validate >>>>>> > > >> > > > > >> not >>>>>> > > >> > > > > >> > > > > stopped node exist and fail test instead of >>>>>> siltent >>>>>> > > >> > implicit >>>>>> > > >> > > > > >> actions. >>>>>> > > >> > > > > >> > > > > >>>>>> > > >> > > > > >> > > > > Sincerely, >>>>>> > > >> > > > > >> > > > > Dmitriy Pavlov >>>>>> > > >> > > > > >> > > > > >>>>>> > > >> > > > > >> > > > > ср, 7 февр. 2018 г. в 13:04, Andrey >>>>>> Kuznetsov < >>>>>> > > >> > > > > stku...@gmail.com >>>>>> > > >> > > > > >> >: >>>>>> > > >> > > > > >> > > > > >>>>>> > > >> > > > > >> > > > > > Hi Maxim, >>>>>> > > >> > > > > >> > > > > > >>>>>> > > >> > > > > >> > > > > > Regarding your first question, the use of >>>>>> > > >> > > afterTestsStopped >>>>>> > > >> > > > is >>>>>> > > >> > > > > >> not >>>>>> > > >> > > > > >> > > > enough >>>>>> > > >> > > > > >> > > > > > to stop all nodes, since each individual >>>>>> test >>>>>> > > >> (method) >>>>>> > > >> > can >>>>>> > > >> > > > > start >>>>>> > > >> > > > > >> > > custom >>>>>> > > >> > > > > >> > > > > set >>>>>> > > >> > > > > >> > > > > > of notes during its operation, and this >>>>>> very test >>>>>> > > >> should >>>>>> > > >> > > > stop >>>>>> > > >> > > > > >> all >>>>>> > > >> > > > > >> > > those >>>>>> > > >> > > > > >> > > > > > nodes. If you delegate closing to >>>>>> > afterTestsStopped >>>>>> > > >> this >>>>>> > > >> > > > will >>>>>> > > >> > > > > >> > affect >>>>>> > > >> > > > > >> > > > only >>>>>> > > >> > > > > >> > > > > > last test (method). Nodes that survive >>>>>> between >>>>>> > > tests >>>>>> > > >> can >>>>>> > > >> > > > > affect >>>>>> > > >> > > > > >> > > > > successive >>>>>> > > >> > > > > >> > > > > > tests. >>>>>> > > >> > > > > >> > > > > > >>>>>> > > >> > > > > >> > > > > > 2018-02-07 1:10 GMT+03:00 Maxim Muzafarov >>>>>> < >>>>>> > > >> > > > maxmu...@gmail.com >>>>>> > > >> > > > > >: >>>>>> > > >> > > > > >> > > > > > >>>>>> > > >> > > > > >> > > > > > > Hello, >>>>>> > > >> > > > > >> > > > > > > >>>>>> > > >> > > > > >> > > > > > > I've made some research about this >>>>>> problem and >>>>>> > i >>>>>> > > >> think >>>>>> > > >> > > > that >>>>>> > > >> > > > > in >>>>>> > > >> > > > > >> > > > general >>>>>> > > >> > > > > >> > > > > we >>>>>> > > >> > > > > >> > > > > > > should move stopAllGrids method in >>>>>> > > GridAbstractTest >>>>>> > > >> > > class >>>>>> > > >> > > > to >>>>>> > > >> > > > > >> > > > > > > afterTestsStopped method with some >>>>>> changes. Am >>>>>> > I >>>>>> > > >> > right? >>>>>> > > >> > > > > >> > > > > > > >>>>>> > > >> > > > > >> > > > > > > Also, I have a question about >>>>>> > > stopAllGrids(boolean >>>>>> > > >> > > cancel) >>>>>> > > >> > > > > >> this >>>>>> > > >> > > > > >> > > > > "cancel" >>>>>> > > >> > > > > >> > > > > > > argument. Why in some cases we should >>>>>> interrupt >>>>>> > > >> > > ComputeJob >>>>>> > > >> > > > > >> and in >>>>>> > > >> > > > > >> > > > some >>>>>> > > >> > > > > >> > > > > > > cases shouldn't? For example here: >>>>>> > > >> > > > > >> > > > > > > >>>>>> > > >> IgniteBaselineAffinityTopologyActivationTest#afterTest >>>>>> > > >> > > > > >> > > > > > > we call method stopAllGrids(false) this >>>>>> way. >>>>>> > Why >>>>>> > > >> not >>>>>> > > >> > > > "true" >>>>>> > > >> > > > > >> > > argument >>>>>> > > >> > > > > >> > > > > > > instead? >>>>>> > > >> > > > > >> > > > > > > >>>>>> > > >> > > > > >> > > > > > > >>>>>> > > >> > > > > >> > > > > > > -- >>>>>> > > >> > > > > >> > > > > > Best regards, >>>>>> > > >> > > > > >> > > > > > Andrey Kuznetsov. >>>>>> > > >> > > > > >> > > > > > >>>>>> > > >> > > > > >> > > > > >>>>>> > > >> > > > > >> > > > >>>>>> > > >> > > > > >> > > >>>>>> > > >> > > > > >> > >>>>>> > > >> > > > > >> >>>>>> > > >> > > > > > >>>>>> > > >> > > > > >>>>>> > > >> > > > >>>>>> > > >> > > >>>>>> > > >> > >>>>>> > > >> >>>>>> > > > >>>>>> > > >>>>>> > >>>>>> >>>>>
