You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Maksim Timonin <ti...@gmail.com> on 2021/03/15 18:58:04 UTC

[DISCUSSION] Patch completely breaks MVCC.

Hi, Igniters!

I'm working on a feature (moving indexes to the core module) and skip
specific implementation for MVCC as it is considered deprecated (the vote
result [1]). Am I right that now there is no need to support MVCC? Then
there are a lot of tests (both Java, C++) that fail because they run with
TRANSACTIONAL_SNAPSHOT atomicity mode.

There are 2 cases:
1. MVCC mode is just a parameter of a test. I just removed it from a
parameters list;
2. There are tests that run only for MVCC. I marked them with the @Ignore
annotation.

But would it better just completely remove all such tests that are broken
by the patch?

[1]
http://apache-ignite-developers.2346864.n4.nabble.com/RESULT-VOTE-Removing-MVCC-public-API-td50705.html#a50706

Re: [DISCUSSION] Patch completely breaks MVCC.

Posted by Maksim Timonin <ti...@gmail.com>.
Folks,

I ran some tests and got a Segmentation Fault without pretty explanation.
It's not actually that user happy to get. Even if there was a warning in
the release notes.
So, I think we should follow the idea of a check for indexes (and cache) on
Ignite start. And looks like designing and testing of the check will take
some time.

But I agree that it can be done within the next minor release. The feature
is experimental for a long enough time and we have a warning in the last
release notes on the first line.

So, for my patch the preferable way is to restore MVCC logic for indexing.
It is cheaper than postponing it while the check and then fixing possible
conflicts.

Sorry, guys.



On Thu, Mar 25, 2021 at 4:59 PM Maxim Muzafarov <mm...@apache.org> wrote:

> Andrey,
>
> > * Ignite shouldn't start if existed persistence has the MVCC index
> (cache) and maybe other internal persistent MVCC structures.
> > * Even if the user dropped all MVCC indices/caches before the upgrade,
> probably there can be an incomplete checkpoint and there are WAL records
> related to MVCC in WAL that should be correctly processed.
>
> This should not be affected if the only MVCC API will be removed. I
> suggest splitting the MVCC removal into 2 separate tasks: public API
> removal + internal MVCC code cleanup.
>
> According to the suggestion above, moving indexes to the core module +
> cleaning up the mvcc API should be enough to move indexes to the core
> module safe.
>
> On Thu, 25 Mar 2021 at 16:14, Maxim Muzafarov <mm...@apache.org> wrote:
> >
> > Folks,
> >
> >
> > I see no reason to postpone this patch. I think we can proceed with
> > this patch with the commitment to remove MVCC from the public API
> > until the next release (2.11). These changes for sure must be
> > well-documented but for the 2.10 release, we already have in the
> > release notes [1] warnings related to MVCC discontinuation.
> >
> > I don't think we should add to the source code some kind of checkers.
> > The MVCC marked with @IgniteExperimental since the 2.9 release, it
> > also mentioned in the 2.10 release notes.
> >
> > [1] https://ignite.apache.org/releases/2.10.0/release_notes.html
> >
> > On Thu, 25 Mar 2021 at 15:59, Maksim Timonin <ti...@gmail.com>
> wrote:
> > >
> > > I've moved indexes from the indexing module to the core module, but I
> did
> > > not move H2Mvcc*IO classes that are responsible for storing MVCC data
> in
> > > indexes. Also for other indexes code I've skipped some blocks with
> > > if-condition that checks if cache is MVCC or not.
> > >
> > > Actually, there isn't so much code to back if we decide to not drop
> MVCC
> > > support for indexes right now. But maybe it is better to implement a
> > > checker that checks that there are still MVCC indexes and fails to
> start
> > > with a link on a migration doc?
> > >
> > >
> > >
> > > On Thu, Mar 25, 2021 at 3:13 PM Alexei Scherbakov <
> > > alexey.scherbakoff@gmail.com> wrote:
> > >
> > > > Maksim,
> > > >
> > > > It seems to me from the description "Patch completely breaks MVCC"
> the
> > > > proposed patch should be postponed until at least the public API for
> > > > MVCC will be removed.
> > > >
> > > > Or can you clarify the impact of the patch ? Does the existing MVCC
> > > > functionality will remain unbroken ?
> > > >
> > > >
> > > > чт, 25 мар. 2021 г. в 14:52, Andrey Mashenkov <
> andrey.mashenkov@gmail.com
> > > > >:
> > > >
> > > > > Hi Maksim,
> > > > >
> > > > > Do you mean MVCC will not work at all or MVCC will not support
> indices
> > > > > after your changes?
> > > > > Anyway, this looks like a major change and may be too harmful for
> the
> > > > minor
> > > > > version (10.1).
> > > > >
> > > > > Before break MVCC index (or MVCC mode) we should force the user
> first to
> > > > > drop all MVCC indices (or even MVCC caches) before switching to the
> > > > version
> > > > > with a fix.
> > > > > The migration process should be well-documented as well.
> > > > >
> > > > > I believe a user should be able to migrate to the new Ignite
> version with
> > > > > exited persistence with no issues. E.g.
> > > > > * Ignite shouldn't start if existed persistence has a MVCC index
> (cache)
> > > > > and maybe other internal persistent MVCC structures.
> > > > > * Even if the user dropped all MVCC indices/caches before the
> upgrade,
> > > > > probably there can be an incomplete checkpoint and there are WAL
> records
> > > > > related to MVCC in WAL that should be correctly processed.
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > On Thu, Mar 25, 2021 at 1:27 PM Maksim Timonin <
> timonin.maxim@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi, Igniters!
> > > > > >
> > > > > > the MVCC feature marked as IgniteExperimental and this
> annotation is
> > > > more
> > > > > > weaker than deprecated. So we can remove this functionality in
> any
> > > > > moment.
> > > > > > So I propose:
> > > > > > 1. Now I leave all affected tests marked as ignored.
> > > > > > 2. Create a ticket for removing TRANSACTIONAL_SNAPSHOT from
> > > > > > CacheAtomicityMode for a future minor release 10.1.
> > > > > > 3. There is a ticket for removing all MVCC code [1]. So we can
> finish
> > > > it
> > > > > in
> > > > > > any release for future.
> > > > > >
> > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-13871
> > > > > >
> > > > > > WDYT?
> > > > > >
> > > > > >
> > > > > > On Mon, Mar 15, 2021 at 9:58 PM Maksim Timonin <
> > > > timonin.maxim@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hi, Igniters!
> > > > > > >
> > > > > > > I'm working on a feature (moving indexes to the core module)
> and skip
> > > > > > > specific implementation for MVCC as it is considered
> deprecated (the
> > > > > vote
> > > > > > > result [1]). Am I right that now there is no need to support
> MVCC?
> > > > Then
> > > > > > > there are a lot of tests (both Java, C++) that fail because
> they run
> > > > > with
> > > > > > > TRANSACTIONAL_SNAPSHOT atomicity mode.
> > > > > > >
> > > > > > > There are 2 cases:
> > > > > > > 1. MVCC mode is just a parameter of a test. I just removed it
> from a
> > > > > > > parameters list;
> > > > > > > 2. There are tests that run only for MVCC. I marked them with
> the
> > > > > @Ignore
> > > > > > > annotation.
> > > > > > >
> > > > > > > But would it better just completely remove all such tests that
> are
> > > > > broken
> > > > > > > by the patch?
> > > > > > >
> > > > > > > [1]
> > > > > > >
> > > > > >
> > > > >
> > > >
> http://apache-ignite-developers.2346864.n4.nabble.com/RESULT-VOTE-Removing-MVCC-public-API-td50705.html#a50706
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Best regards,
> > > > > Andrey V. Mashenkov
> > > > >
> > > >
> > > >
> > > > --
> > > >
> > > > Best regards,
> > > > Alexei Scherbakov
> > > >
>

Re: [DISCUSSION] Patch completely breaks MVCC.

Posted by Maxim Muzafarov <mm...@apache.org>.
Andrey,

> * Ignite shouldn't start if existed persistence has the MVCC index (cache) and maybe other internal persistent MVCC structures.
> * Even if the user dropped all MVCC indices/caches before the upgrade, probably there can be an incomplete checkpoint and there are WAL records related to MVCC in WAL that should be correctly processed.

This should not be affected if the only MVCC API will be removed. I
suggest splitting the MVCC removal into 2 separate tasks: public API
removal + internal MVCC code cleanup.

According to the suggestion above, moving indexes to the core module +
cleaning up the mvcc API should be enough to move indexes to the core
module safe.

On Thu, 25 Mar 2021 at 16:14, Maxim Muzafarov <mm...@apache.org> wrote:
>
> Folks,
>
>
> I see no reason to postpone this patch. I think we can proceed with
> this patch with the commitment to remove MVCC from the public API
> until the next release (2.11). These changes for sure must be
> well-documented but for the 2.10 release, we already have in the
> release notes [1] warnings related to MVCC discontinuation.
>
> I don't think we should add to the source code some kind of checkers.
> The MVCC marked with @IgniteExperimental since the 2.9 release, it
> also mentioned in the 2.10 release notes.
>
> [1] https://ignite.apache.org/releases/2.10.0/release_notes.html
>
> On Thu, 25 Mar 2021 at 15:59, Maksim Timonin <ti...@gmail.com> wrote:
> >
> > I've moved indexes from the indexing module to the core module, but I did
> > not move H2Mvcc*IO classes that are responsible for storing MVCC data in
> > indexes. Also for other indexes code I've skipped some blocks with
> > if-condition that checks if cache is MVCC or not.
> >
> > Actually, there isn't so much code to back if we decide to not drop MVCC
> > support for indexes right now. But maybe it is better to implement a
> > checker that checks that there are still MVCC indexes and fails to start
> > with a link on a migration doc?
> >
> >
> >
> > On Thu, Mar 25, 2021 at 3:13 PM Alexei Scherbakov <
> > alexey.scherbakoff@gmail.com> wrote:
> >
> > > Maksim,
> > >
> > > It seems to me from the description "Patch completely breaks MVCC" the
> > > proposed patch should be postponed until at least the public API for
> > > MVCC will be removed.
> > >
> > > Or can you clarify the impact of the patch ? Does the existing MVCC
> > > functionality will remain unbroken ?
> > >
> > >
> > > чт, 25 мар. 2021 г. в 14:52, Andrey Mashenkov <andrey.mashenkov@gmail.com
> > > >:
> > >
> > > > Hi Maksim,
> > > >
> > > > Do you mean MVCC will not work at all or MVCC will not support indices
> > > > after your changes?
> > > > Anyway, this looks like a major change and may be too harmful for the
> > > minor
> > > > version (10.1).
> > > >
> > > > Before break MVCC index (or MVCC mode) we should force the user first to
> > > > drop all MVCC indices (or even MVCC caches) before switching to the
> > > version
> > > > with a fix.
> > > > The migration process should be well-documented as well.
> > > >
> > > > I believe a user should be able to migrate to the new Ignite version with
> > > > exited persistence with no issues. E.g.
> > > > * Ignite shouldn't start if existed persistence has a MVCC index (cache)
> > > > and maybe other internal persistent MVCC structures.
> > > > * Even if the user dropped all MVCC indices/caches before the upgrade,
> > > > probably there can be an incomplete checkpoint and there are WAL records
> > > > related to MVCC in WAL that should be correctly processed.
> > > >
> > > >
> > > >
> > > >
> > > > On Thu, Mar 25, 2021 at 1:27 PM Maksim Timonin <ti...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi, Igniters!
> > > > >
> > > > > the MVCC feature marked as IgniteExperimental and this annotation is
> > > more
> > > > > weaker than deprecated. So we can remove this functionality in any
> > > > moment.
> > > > > So I propose:
> > > > > 1. Now I leave all affected tests marked as ignored.
> > > > > 2. Create a ticket for removing TRANSACTIONAL_SNAPSHOT from
> > > > > CacheAtomicityMode for a future minor release 10.1.
> > > > > 3. There is a ticket for removing all MVCC code [1]. So we can finish
> > > it
> > > > in
> > > > > any release for future.
> > > > >
> > > > > [1] https://issues.apache.org/jira/browse/IGNITE-13871
> > > > >
> > > > > WDYT?
> > > > >
> > > > >
> > > > > On Mon, Mar 15, 2021 at 9:58 PM Maksim Timonin <
> > > timonin.maxim@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi, Igniters!
> > > > > >
> > > > > > I'm working on a feature (moving indexes to the core module) and skip
> > > > > > specific implementation for MVCC as it is considered deprecated (the
> > > > vote
> > > > > > result [1]). Am I right that now there is no need to support MVCC?
> > > Then
> > > > > > there are a lot of tests (both Java, C++) that fail because they run
> > > > with
> > > > > > TRANSACTIONAL_SNAPSHOT atomicity mode.
> > > > > >
> > > > > > There are 2 cases:
> > > > > > 1. MVCC mode is just a parameter of a test. I just removed it from a
> > > > > > parameters list;
> > > > > > 2. There are tests that run only for MVCC. I marked them with the
> > > > @Ignore
> > > > > > annotation.
> > > > > >
> > > > > > But would it better just completely remove all such tests that are
> > > > broken
> > > > > > by the patch?
> > > > > >
> > > > > > [1]
> > > > > >
> > > > >
> > > >
> > > http://apache-ignite-developers.2346864.n4.nabble.com/RESULT-VOTE-Removing-MVCC-public-API-td50705.html#a50706
> > > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > > Best regards,
> > > > Andrey V. Mashenkov
> > > >
> > >
> > >
> > > --
> > >
> > > Best regards,
> > > Alexei Scherbakov
> > >

Re: [DISCUSSION] Patch completely breaks MVCC.

Posted by Maxim Muzafarov <mm...@apache.org>.
Folks,


I see no reason to postpone this patch. I think we can proceed with
this patch with the commitment to remove MVCC from the public API
until the next release (2.11). These changes for sure must be
well-documented but for the 2.10 release, we already have in the
release notes [1] warnings related to MVCC discontinuation.

I don't think we should add to the source code some kind of checkers.
The MVCC marked with @IgniteExperimental since the 2.9 release, it
also mentioned in the 2.10 release notes.

[1] https://ignite.apache.org/releases/2.10.0/release_notes.html

On Thu, 25 Mar 2021 at 15:59, Maksim Timonin <ti...@gmail.com> wrote:
>
> I've moved indexes from the indexing module to the core module, but I did
> not move H2Mvcc*IO classes that are responsible for storing MVCC data in
> indexes. Also for other indexes code I've skipped some blocks with
> if-condition that checks if cache is MVCC or not.
>
> Actually, there isn't so much code to back if we decide to not drop MVCC
> support for indexes right now. But maybe it is better to implement a
> checker that checks that there are still MVCC indexes and fails to start
> with a link on a migration doc?
>
>
>
> On Thu, Mar 25, 2021 at 3:13 PM Alexei Scherbakov <
> alexey.scherbakoff@gmail.com> wrote:
>
> > Maksim,
> >
> > It seems to me from the description "Patch completely breaks MVCC" the
> > proposed patch should be postponed until at least the public API for
> > MVCC will be removed.
> >
> > Or can you clarify the impact of the patch ? Does the existing MVCC
> > functionality will remain unbroken ?
> >
> >
> > чт, 25 мар. 2021 г. в 14:52, Andrey Mashenkov <andrey.mashenkov@gmail.com
> > >:
> >
> > > Hi Maksim,
> > >
> > > Do you mean MVCC will not work at all or MVCC will not support indices
> > > after your changes?
> > > Anyway, this looks like a major change and may be too harmful for the
> > minor
> > > version (10.1).
> > >
> > > Before break MVCC index (or MVCC mode) we should force the user first to
> > > drop all MVCC indices (or even MVCC caches) before switching to the
> > version
> > > with a fix.
> > > The migration process should be well-documented as well.
> > >
> > > I believe a user should be able to migrate to the new Ignite version with
> > > exited persistence with no issues. E.g.
> > > * Ignite shouldn't start if existed persistence has a MVCC index (cache)
> > > and maybe other internal persistent MVCC structures.
> > > * Even if the user dropped all MVCC indices/caches before the upgrade,
> > > probably there can be an incomplete checkpoint and there are WAL records
> > > related to MVCC in WAL that should be correctly processed.
> > >
> > >
> > >
> > >
> > > On Thu, Mar 25, 2021 at 1:27 PM Maksim Timonin <ti...@gmail.com>
> > > wrote:
> > >
> > > > Hi, Igniters!
> > > >
> > > > the MVCC feature marked as IgniteExperimental and this annotation is
> > more
> > > > weaker than deprecated. So we can remove this functionality in any
> > > moment.
> > > > So I propose:
> > > > 1. Now I leave all affected tests marked as ignored.
> > > > 2. Create a ticket for removing TRANSACTIONAL_SNAPSHOT from
> > > > CacheAtomicityMode for a future minor release 10.1.
> > > > 3. There is a ticket for removing all MVCC code [1]. So we can finish
> > it
> > > in
> > > > any release for future.
> > > >
> > > > [1] https://issues.apache.org/jira/browse/IGNITE-13871
> > > >
> > > > WDYT?
> > > >
> > > >
> > > > On Mon, Mar 15, 2021 at 9:58 PM Maksim Timonin <
> > timonin.maxim@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi, Igniters!
> > > > >
> > > > > I'm working on a feature (moving indexes to the core module) and skip
> > > > > specific implementation for MVCC as it is considered deprecated (the
> > > vote
> > > > > result [1]). Am I right that now there is no need to support MVCC?
> > Then
> > > > > there are a lot of tests (both Java, C++) that fail because they run
> > > with
> > > > > TRANSACTIONAL_SNAPSHOT atomicity mode.
> > > > >
> > > > > There are 2 cases:
> > > > > 1. MVCC mode is just a parameter of a test. I just removed it from a
> > > > > parameters list;
> > > > > 2. There are tests that run only for MVCC. I marked them with the
> > > @Ignore
> > > > > annotation.
> > > > >
> > > > > But would it better just completely remove all such tests that are
> > > broken
> > > > > by the patch?
> > > > >
> > > > > [1]
> > > > >
> > > >
> > >
> > http://apache-ignite-developers.2346864.n4.nabble.com/RESULT-VOTE-Removing-MVCC-public-API-td50705.html#a50706
> > > > >
> > > >
> > >
> > >
> > > --
> > > Best regards,
> > > Andrey V. Mashenkov
> > >
> >
> >
> > --
> >
> > Best regards,
> > Alexei Scherbakov
> >

Re: [DISCUSSION] Patch completely breaks MVCC.

Posted by Maksim Timonin <ti...@gmail.com>.
I've moved indexes from the indexing module to the core module, but I did
not move H2Mvcc*IO classes that are responsible for storing MVCC data in
indexes. Also for other indexes code I've skipped some blocks with
if-condition that checks if cache is MVCC or not.

Actually, there isn't so much code to back if we decide to not drop MVCC
support for indexes right now. But maybe it is better to implement a
checker that checks that there are still MVCC indexes and fails to start
with a link on a migration doc?



On Thu, Mar 25, 2021 at 3:13 PM Alexei Scherbakov <
alexey.scherbakoff@gmail.com> wrote:

> Maksim,
>
> It seems to me from the description "Patch completely breaks MVCC" the
> proposed patch should be postponed until at least the public API for
> MVCC will be removed.
>
> Or can you clarify the impact of the patch ? Does the existing MVCC
> functionality will remain unbroken ?
>
>
> чт, 25 мар. 2021 г. в 14:52, Andrey Mashenkov <andrey.mashenkov@gmail.com
> >:
>
> > Hi Maksim,
> >
> > Do you mean MVCC will not work at all or MVCC will not support indices
> > after your changes?
> > Anyway, this looks like a major change and may be too harmful for the
> minor
> > version (10.1).
> >
> > Before break MVCC index (or MVCC mode) we should force the user first to
> > drop all MVCC indices (or even MVCC caches) before switching to the
> version
> > with a fix.
> > The migration process should be well-documented as well.
> >
> > I believe a user should be able to migrate to the new Ignite version with
> > exited persistence with no issues. E.g.
> > * Ignite shouldn't start if existed persistence has a MVCC index (cache)
> > and maybe other internal persistent MVCC structures.
> > * Even if the user dropped all MVCC indices/caches before the upgrade,
> > probably there can be an incomplete checkpoint and there are WAL records
> > related to MVCC in WAL that should be correctly processed.
> >
> >
> >
> >
> > On Thu, Mar 25, 2021 at 1:27 PM Maksim Timonin <ti...@gmail.com>
> > wrote:
> >
> > > Hi, Igniters!
> > >
> > > the MVCC feature marked as IgniteExperimental and this annotation is
> more
> > > weaker than deprecated. So we can remove this functionality in any
> > moment.
> > > So I propose:
> > > 1. Now I leave all affected tests marked as ignored.
> > > 2. Create a ticket for removing TRANSACTIONAL_SNAPSHOT from
> > > CacheAtomicityMode for a future minor release 10.1.
> > > 3. There is a ticket for removing all MVCC code [1]. So we can finish
> it
> > in
> > > any release for future.
> > >
> > > [1] https://issues.apache.org/jira/browse/IGNITE-13871
> > >
> > > WDYT?
> > >
> > >
> > > On Mon, Mar 15, 2021 at 9:58 PM Maksim Timonin <
> timonin.maxim@gmail.com>
> > > wrote:
> > >
> > > > Hi, Igniters!
> > > >
> > > > I'm working on a feature (moving indexes to the core module) and skip
> > > > specific implementation for MVCC as it is considered deprecated (the
> > vote
> > > > result [1]). Am I right that now there is no need to support MVCC?
> Then
> > > > there are a lot of tests (both Java, C++) that fail because they run
> > with
> > > > TRANSACTIONAL_SNAPSHOT atomicity mode.
> > > >
> > > > There are 2 cases:
> > > > 1. MVCC mode is just a parameter of a test. I just removed it from a
> > > > parameters list;
> > > > 2. There are tests that run only for MVCC. I marked them with the
> > @Ignore
> > > > annotation.
> > > >
> > > > But would it better just completely remove all such tests that are
> > broken
> > > > by the patch?
> > > >
> > > > [1]
> > > >
> > >
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/RESULT-VOTE-Removing-MVCC-public-API-td50705.html#a50706
> > > >
> > >
> >
> >
> > --
> > Best regards,
> > Andrey V. Mashenkov
> >
>
>
> --
>
> Best regards,
> Alexei Scherbakov
>

Re: [DISCUSSION] Patch completely breaks MVCC.

Posted by Alexei Scherbakov <al...@gmail.com>.
Maksim,

It seems to me from the description "Patch completely breaks MVCC" the
proposed patch should be postponed until at least the public API for
MVCC will be removed.

Or can you clarify the impact of the patch ? Does the existing MVCC
functionality will remain unbroken ?


чт, 25 мар. 2021 г. в 14:52, Andrey Mashenkov <an...@gmail.com>:

> Hi Maksim,
>
> Do you mean MVCC will not work at all or MVCC will not support indices
> after your changes?
> Anyway, this looks like a major change and may be too harmful for the minor
> version (10.1).
>
> Before break MVCC index (or MVCC mode) we should force the user first to
> drop all MVCC indices (or even MVCC caches) before switching to the version
> with a fix.
> The migration process should be well-documented as well.
>
> I believe a user should be able to migrate to the new Ignite version with
> exited persistence with no issues. E.g.
> * Ignite shouldn't start if existed persistence has a MVCC index (cache)
> and maybe other internal persistent MVCC structures.
> * Even if the user dropped all MVCC indices/caches before the upgrade,
> probably there can be an incomplete checkpoint and there are WAL records
> related to MVCC in WAL that should be correctly processed.
>
>
>
>
> On Thu, Mar 25, 2021 at 1:27 PM Maksim Timonin <ti...@gmail.com>
> wrote:
>
> > Hi, Igniters!
> >
> > the MVCC feature marked as IgniteExperimental and this annotation is more
> > weaker than deprecated. So we can remove this functionality in any
> moment.
> > So I propose:
> > 1. Now I leave all affected tests marked as ignored.
> > 2. Create a ticket for removing TRANSACTIONAL_SNAPSHOT from
> > CacheAtomicityMode for a future minor release 10.1.
> > 3. There is a ticket for removing all MVCC code [1]. So we can finish it
> in
> > any release for future.
> >
> > [1] https://issues.apache.org/jira/browse/IGNITE-13871
> >
> > WDYT?
> >
> >
> > On Mon, Mar 15, 2021 at 9:58 PM Maksim Timonin <ti...@gmail.com>
> > wrote:
> >
> > > Hi, Igniters!
> > >
> > > I'm working on a feature (moving indexes to the core module) and skip
> > > specific implementation for MVCC as it is considered deprecated (the
> vote
> > > result [1]). Am I right that now there is no need to support MVCC? Then
> > > there are a lot of tests (both Java, C++) that fail because they run
> with
> > > TRANSACTIONAL_SNAPSHOT atomicity mode.
> > >
> > > There are 2 cases:
> > > 1. MVCC mode is just a parameter of a test. I just removed it from a
> > > parameters list;
> > > 2. There are tests that run only for MVCC. I marked them with the
> @Ignore
> > > annotation.
> > >
> > > But would it better just completely remove all such tests that are
> broken
> > > by the patch?
> > >
> > > [1]
> > >
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/RESULT-VOTE-Removing-MVCC-public-API-td50705.html#a50706
> > >
> >
>
>
> --
> Best regards,
> Andrey V. Mashenkov
>


-- 

Best regards,
Alexei Scherbakov

Re: [DISCUSSION] Patch completely breaks MVCC.

Posted by Andrey Mashenkov <an...@gmail.com>.
Hi Maksim,

Do you mean MVCC will not work at all or MVCC will not support indices
after your changes?
Anyway, this looks like a major change and may be too harmful for the minor
version (10.1).

Before break MVCC index (or MVCC mode) we should force the user first to
drop all MVCC indices (or even MVCC caches) before switching to the version
with a fix.
The migration process should be well-documented as well.

I believe a user should be able to migrate to the new Ignite version with
exited persistence with no issues. E.g.
* Ignite shouldn't start if existed persistence has a MVCC index (cache)
and maybe other internal persistent MVCC structures.
* Even if the user dropped all MVCC indices/caches before the upgrade,
probably there can be an incomplete checkpoint and there are WAL records
related to MVCC in WAL that should be correctly processed.




On Thu, Mar 25, 2021 at 1:27 PM Maksim Timonin <ti...@gmail.com>
wrote:

> Hi, Igniters!
>
> the MVCC feature marked as IgniteExperimental and this annotation is more
> weaker than deprecated. So we can remove this functionality in any moment.
> So I propose:
> 1. Now I leave all affected tests marked as ignored.
> 2. Create a ticket for removing TRANSACTIONAL_SNAPSHOT from
> CacheAtomicityMode for a future minor release 10.1.
> 3. There is a ticket for removing all MVCC code [1]. So we can finish it in
> any release for future.
>
> [1] https://issues.apache.org/jira/browse/IGNITE-13871
>
> WDYT?
>
>
> On Mon, Mar 15, 2021 at 9:58 PM Maksim Timonin <ti...@gmail.com>
> wrote:
>
> > Hi, Igniters!
> >
> > I'm working on a feature (moving indexes to the core module) and skip
> > specific implementation for MVCC as it is considered deprecated (the vote
> > result [1]). Am I right that now there is no need to support MVCC? Then
> > there are a lot of tests (both Java, C++) that fail because they run with
> > TRANSACTIONAL_SNAPSHOT atomicity mode.
> >
> > There are 2 cases:
> > 1. MVCC mode is just a parameter of a test. I just removed it from a
> > parameters list;
> > 2. There are tests that run only for MVCC. I marked them with the @Ignore
> > annotation.
> >
> > But would it better just completely remove all such tests that are broken
> > by the patch?
> >
> > [1]
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/RESULT-VOTE-Removing-MVCC-public-API-td50705.html#a50706
> >
>


-- 
Best regards,
Andrey V. Mashenkov

Re: [DISCUSSION] Patch completely breaks MVCC.

Posted by Maksim Timonin <ti...@gmail.com>.
Hi, Igniters!

the MVCC feature marked as IgniteExperimental and this annotation is more
weaker than deprecated. So we can remove this functionality in any moment.
So I propose:
1. Now I leave all affected tests marked as ignored.
2. Create a ticket for removing TRANSACTIONAL_SNAPSHOT from
CacheAtomicityMode for a future minor release 10.1.
3. There is a ticket for removing all MVCC code [1]. So we can finish it in
any release for future.

[1] https://issues.apache.org/jira/browse/IGNITE-13871

WDYT?


On Mon, Mar 15, 2021 at 9:58 PM Maksim Timonin <ti...@gmail.com>
wrote:

> Hi, Igniters!
>
> I'm working on a feature (moving indexes to the core module) and skip
> specific implementation for MVCC as it is considered deprecated (the vote
> result [1]). Am I right that now there is no need to support MVCC? Then
> there are a lot of tests (both Java, C++) that fail because they run with
> TRANSACTIONAL_SNAPSHOT atomicity mode.
>
> There are 2 cases:
> 1. MVCC mode is just a parameter of a test. I just removed it from a
> parameters list;
> 2. There are tests that run only for MVCC. I marked them with the @Ignore
> annotation.
>
> But would it better just completely remove all such tests that are broken
> by the patch?
>
> [1]
> http://apache-ignite-developers.2346864.n4.nabble.com/RESULT-VOTE-Removing-MVCC-public-API-td50705.html#a50706
>