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.mashen...@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
