You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Andrey Mashenkov <an...@gmail.com> on 2018/11/21 14:57:10 UTC
MVCC test coverage.
Hi Igniters,
As you may already know, MVCC transaction feature will be available in
upcoming Ignite-2.7.
However, MVCC Tx feature is released for beta testing and has many
limitations and we a going
to improve stability and integrations with other features in future
releases.
We can reuse existed transactional cache tests and run them in Mvcc mode to
get much better test coverage with small looses.
Here is a ticket for this IGNITE-10001 [1].
This mean we will have twice more "Cache Tests" and get TC runs some longer.
To reduce new Mvcc cache suites impact and save TC time we are going to
1. Include new tests to nightly suite only, this will allow us to put our
ears to the ground.
2. Exclude non-tx tests and non-relevant tx cases and aggressively mute
tests for unsupported features integrations.
I've implement a PR to one of tasks [2] as an example how it can be done.
Technical details:
1. Introduced a new FORCE_MVCC flag and created a child "Mvcc Cache 2"
suite for "Cache 2" test suite with FORCE_MVCC flag on.
2. Implemented a hook that change TRANSACTIONAL cache atomicity mode to
TRANSACTIONAL_SNAPSHOT if FORCE_MVCC flag turned on.
This allow us to check MVCC mode without creating new test classes and
minimal intervention in existed tests code.
3. All irrelevant tests marked as ignored in Mvcc suite.
4. Known unsupported cases are muted. New failures muted as well
(corresponding tickets were created).
Any pros\cons?
Can someone please review a PR?
[1] https://issues.apache.org/jira/browse/IGNITE-10001
[2] https://issues.apache.org/jira/browse/IGNITE-10002
--
Best regards,
Andrey V. Mashenkov
Re: MVCC test coverage.
Posted by Vladimir Ozerov <vo...@gridgain.com>.
Hi Roman,
Agree with your suggestions. Regarding the problem 2 - note that we added
cache mode valuation to MVCC suites. So yes, we may have some duplication,
but it will be revealed on the next nightly test run.
Not convenient, but at the very least we will know about it. Though, I am
pretty sure we will become annoyed with constant failures very fast. So for
sure this is a temporal solution.
Vladimir.
вт, 27 нояб. 2018 г. в 14:03, Roman Kondakov <ko...@mail.ru.invalid>:
> Vladimir, Andrey,
>
> as you mentioned this approach has several disadvantages. I can name a
> few of them:
>
> 1. This new MVCC suites will be triggered in "long" runs at night - this
> means developers will not receive feedback about MVCC problems
> immediately - they will have to wait until their commit will be merged
> to master and than triggered at night build. It may lead to permanent
> problems in master branch.
>
> 2. Developers should always keep in mind that all tests they add will be
> run twice: for MVCC mode and non-MVCC mode. And if they don't want their
> tests run twice, such tests should be added to exclude map in the
> according MVCC suite. Due to the fact this is not the obvious rule and
> we do not have any possibility to control this process, I expect this
> rule will be violated very often. This lead us to double runs of non
> MVCC-relevant tests.
>
> 3. MVCC has became a full-fledged feature of Apache Ignite. Each
> developer should take it into account when contributing to project. Mvcc
> case should be considered in each feature as well as other atomicity
> modes: transactional and atomic. Proposed approach removes the need for
> the developer to think of MVCC at all. Everybody will assume that if
> they've added atomic and transactional tests, their job is done, beacuse
> MVCC test should run automatically. IMO this is not good.
>
>
> Of course, proposed approach has an obvious advantage: it is very fast.
> We can adopt old tests to MVCC case in a couple weeks. So, it is a good
> temporary solution.
>
> As a possible long-term solution I would propose the following:
>
> 1. Do not inherit MVCC suites from non-MVCC suites, but instead refactor
> it - i.e. extract common logic to the basic abstract class and run this
> tests with different atomicity modes - MVCC and non-MVCC.
>
> 2. Notify developers about TRANSACTIONAL_SNAPSHOT atomicity mode has the
> same importance as other modes and it should be considered in the same
> way as other.
>
> 3. To deal with the dramatically increased number of tests, RunAll suite
> could be split into two variants: RunAll(full) and RunAll(fast) as
> discussed on dev-list several times. Full suite runs all tests, fast
> suite runs only a subset of tests (or all tests but with the smaller
> timeouts -it is under discussion). One of the proposed ways [1] - is to
> extract only significant, representative tests from the entire suite,
> and run this small subset on "fast" RunAll's. In this case if we have a
> significant test in MVCC suite - we do not have to wait night build
> until this test is checked - because if the test is significant - it in
> the "fast" run by default.
>
>
> [1]
>
> http://apache-ignite-developers.2346864.n4.nabble.com/Brainstorm-Make-TC-Run-All-faster-tt37845.html#a38445
>
> --
>
> Kind Regards
> Roman Kondakov
>
> On 21.11.2018 22:37, Vladimir Ozerov wrote:
> > Hi Andrey,
> >
> > Thank you for bringing this question to the list. I already reviewed this
> > PR and it looks good to me. But I would like to hear more opinions from
> > other community members regarding the whole approach.
> >
> > One important detail - we are going to create new suites as a child
> classes
> > of existing suites with irrelevant tests excluded manually. This way if a
> > new test is added to existing cache suite, it will be automatically added
> > to TC suite as well, and we will see potential MVCC issues in a nightly
> > build. This is critical thing to keep MVCC mode on par with “classical”
> > transactions.
> >
> > I am not 100% happy with the fact that we will know about new failures
> only
> > after problematic commit is pushed. But I do not see how to improve it
> > without extending Run All time for another 30 hours. This will do more
> harm
> > than good. So proposed solution looks like a good pros-cons balance at
> the
> > moment.
> >
> > Vladimir.
> >
> >
> >
> > ср, 21 нояб. 2018 г. в 17:59, Andrey Mashenkov <
> andrey.mashenkov@gmail.com>:
> >
> >> Hi Igniters,
> >>
> >> As you may already know, MVCC transaction feature will be available in
> >> upcoming Ignite-2.7.
> >> However, MVCC Tx feature is released for beta testing and has many
> >> limitations and we a going
> >> to improve stability and integrations with other features in future
> >> releases.
> >>
> >> We can reuse existed transactional cache tests and run them in Mvcc
> mode to
> >> get much better test coverage with small looses.
> >> Here is a ticket for this IGNITE-10001 [1].
> >>
> >> This mean we will have twice more "Cache Tests" and get TC runs some
> >> longer.
> >> To reduce new Mvcc cache suites impact and save TC time we are going to
> >> 1. Include new tests to nightly suite only, this will allow us to put
> our
> >> ears to the ground.
> >> 2. Exclude non-tx tests and non-relevant tx cases and aggressively mute
> >> tests for unsupported features integrations.
> >>
> >> I've implement a PR to one of tasks [2] as an example how it can be
> done.
> >>
> >> Technical details:
> >> 1. Introduced a new FORCE_MVCC flag and created a child "Mvcc Cache 2"
> >> suite for "Cache 2" test suite with FORCE_MVCC flag on.
> >> 2. Implemented a hook that change TRANSACTIONAL cache atomicity mode to
> >> TRANSACTIONAL_SNAPSHOT if FORCE_MVCC flag turned on.
> >> This allow us to check MVCC mode without creating new test classes and
> >> minimal intervention in existed tests code.
> >> 3. All irrelevant tests marked as ignored in Mvcc suite.
> >> 4. Known unsupported cases are muted. New failures muted as well
> >> (corresponding tickets were created).
> >>
> >>
> >> Any pros\cons?
> >> Can someone please review a PR?
> >>
> >> [1] https://issues.apache.org/jira/browse/IGNITE-10001
> >> [2] https://issues.apache.org/jira/browse/IGNITE-10002
> >> --
> >> Best regards,
> >> Andrey V. Mashenkov
> >>
>
Re: MVCC test coverage.
Posted by Roman Kondakov <ko...@mail.ru.INVALID>.
Vladimir, Andrey,
as you mentioned this approach has several disadvantages. I can name a
few of them:
1. This new MVCC suites will be triggered in "long" runs at night - this
means developers will not receive feedback about MVCC problems
immediately - they will have to wait until their commit will be merged
to master and than triggered at night build. It may lead to permanent
problems in master branch.
2. Developers should always keep in mind that all tests they add will be
run twice: for MVCC mode and non-MVCC mode. And if they don't want their
tests run twice, such tests should be added to exclude map in the
according MVCC suite. Due to the fact this is not the obvious rule and
we do not have any possibility to control this process, I expect this
rule will be violated very often. This lead us to double runs of non
MVCC-relevant tests.
3. MVCC has became a full-fledged feature of Apache Ignite. Each
developer should take it into account when contributing to project. Mvcc
case should be considered in each feature as well as other atomicity
modes: transactional and atomic. Proposed approach removes the need for
the developer to think of MVCC at all. Everybody will assume that if
they've added atomic and transactional tests, their job is done, beacuse
MVCC test should run automatically. IMO this is not good.
Of course, proposed approach has an obvious advantage: it is very fast.
We can adopt old tests to MVCC case in a couple weeks. So, it is a good
temporary solution.
As a possible long-term solution I would propose the following:
1. Do not inherit MVCC suites from non-MVCC suites, but instead refactor
it - i.e. extract common logic to the basic abstract class and run this
tests with different atomicity modes - MVCC and non-MVCC.
2. Notify developers about TRANSACTIONAL_SNAPSHOT atomicity mode has the
same importance as other modes and it should be considered in the same
way as other.
3. To deal with the dramatically increased number of tests, RunAll suite
could be split into two variants: RunAll(full) and RunAll(fast) as
discussed on dev-list several times. Full suite runs all tests, fast
suite runs only a subset of tests (or all tests but with the smaller
timeouts -it is under discussion). One of the proposed ways [1] - is to
extract only significant, representative tests from the entire suite,
and run this small subset on "fast" RunAll's. In this case if we have a
significant test in MVCC suite - we do not have to wait night build
until this test is checked - because if the test is significant - it in
the "fast" run by default.
[1]
http://apache-ignite-developers.2346864.n4.nabble.com/Brainstorm-Make-TC-Run-All-faster-tt37845.html#a38445
--
Kind Regards
Roman Kondakov
On 21.11.2018 22:37, Vladimir Ozerov wrote:
> Hi Andrey,
>
> Thank you for bringing this question to the list. I already reviewed this
> PR and it looks good to me. But I would like to hear more opinions from
> other community members regarding the whole approach.
>
> One important detail - we are going to create new suites as a child classes
> of existing suites with irrelevant tests excluded manually. This way if a
> new test is added to existing cache suite, it will be automatically added
> to TC suite as well, and we will see potential MVCC issues in a nightly
> build. This is critical thing to keep MVCC mode on par with “classical”
> transactions.
>
> I am not 100% happy with the fact that we will know about new failures only
> after problematic commit is pushed. But I do not see how to improve it
> without extending Run All time for another 30 hours. This will do more harm
> than good. So proposed solution looks like a good pros-cons balance at the
> moment.
>
> Vladimir.
>
>
>
> ср, 21 нояб. 2018 г. в 17:59, Andrey Mashenkov <an...@gmail.com>:
>
>> Hi Igniters,
>>
>> As you may already know, MVCC transaction feature will be available in
>> upcoming Ignite-2.7.
>> However, MVCC Tx feature is released for beta testing and has many
>> limitations and we a going
>> to improve stability and integrations with other features in future
>> releases.
>>
>> We can reuse existed transactional cache tests and run them in Mvcc mode to
>> get much better test coverage with small looses.
>> Here is a ticket for this IGNITE-10001 [1].
>>
>> This mean we will have twice more "Cache Tests" and get TC runs some
>> longer.
>> To reduce new Mvcc cache suites impact and save TC time we are going to
>> 1. Include new tests to nightly suite only, this will allow us to put our
>> ears to the ground.
>> 2. Exclude non-tx tests and non-relevant tx cases and aggressively mute
>> tests for unsupported features integrations.
>>
>> I've implement a PR to one of tasks [2] as an example how it can be done.
>>
>> Technical details:
>> 1. Introduced a new FORCE_MVCC flag and created a child "Mvcc Cache 2"
>> suite for "Cache 2" test suite with FORCE_MVCC flag on.
>> 2. Implemented a hook that change TRANSACTIONAL cache atomicity mode to
>> TRANSACTIONAL_SNAPSHOT if FORCE_MVCC flag turned on.
>> This allow us to check MVCC mode without creating new test classes and
>> minimal intervention in existed tests code.
>> 3. All irrelevant tests marked as ignored in Mvcc suite.
>> 4. Known unsupported cases are muted. New failures muted as well
>> (corresponding tickets were created).
>>
>>
>> Any pros\cons?
>> Can someone please review a PR?
>>
>> [1] https://issues.apache.org/jira/browse/IGNITE-10001
>> [2] https://issues.apache.org/jira/browse/IGNITE-10002
>> --
>> Best regards,
>> Andrey V. Mashenkov
>>
Re: MVCC test coverage.
Posted by Vladimir Ozerov <vo...@gridgain.com>.
Hi Andrey,
Thank you for bringing this question to the list. I already reviewed this
PR and it looks good to me. But I would like to hear more opinions from
other community members regarding the whole approach.
One important detail - we are going to create new suites as a child classes
of existing suites with irrelevant tests excluded manually. This way if a
new test is added to existing cache suite, it will be automatically added
to TC suite as well, and we will see potential MVCC issues in a nightly
build. This is critical thing to keep MVCC mode on par with “classical”
transactions.
I am not 100% happy with the fact that we will know about new failures only
after problematic commit is pushed. But I do not see how to improve it
without extending Run All time for another 30 hours. This will do more harm
than good. So proposed solution looks like a good pros-cons balance at the
moment.
Vladimir.
ср, 21 нояб. 2018 г. в 17:59, Andrey Mashenkov <an...@gmail.com>:
> Hi Igniters,
>
> As you may already know, MVCC transaction feature will be available in
> upcoming Ignite-2.7.
> However, MVCC Tx feature is released for beta testing and has many
> limitations and we a going
> to improve stability and integrations with other features in future
> releases.
>
> We can reuse existed transactional cache tests and run them in Mvcc mode to
> get much better test coverage with small looses.
> Here is a ticket for this IGNITE-10001 [1].
>
> This mean we will have twice more "Cache Tests" and get TC runs some
> longer.
> To reduce new Mvcc cache suites impact and save TC time we are going to
> 1. Include new tests to nightly suite only, this will allow us to put our
> ears to the ground.
> 2. Exclude non-tx tests and non-relevant tx cases and aggressively mute
> tests for unsupported features integrations.
>
> I've implement a PR to one of tasks [2] as an example how it can be done.
>
> Technical details:
> 1. Introduced a new FORCE_MVCC flag and created a child "Mvcc Cache 2"
> suite for "Cache 2" test suite with FORCE_MVCC flag on.
> 2. Implemented a hook that change TRANSACTIONAL cache atomicity mode to
> TRANSACTIONAL_SNAPSHOT if FORCE_MVCC flag turned on.
> This allow us to check MVCC mode without creating new test classes and
> minimal intervention in existed tests code.
> 3. All irrelevant tests marked as ignored in Mvcc suite.
> 4. Known unsupported cases are muted. New failures muted as well
> (corresponding tickets were created).
>
>
> Any pros\cons?
> Can someone please review a PR?
>
> [1] https://issues.apache.org/jira/browse/IGNITE-10001
> [2] https://issues.apache.org/jira/browse/IGNITE-10002
> --
> Best regards,
> Andrey V. Mashenkov
>