ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Roman Kondakov <kondako...@mail.ru.INVALID>
Subject Re: MVCC test coverage.
Date Tue, 27 Nov 2018 11:02:49 GMT
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
>>

Mime
View raw message