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 <mmu...@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 <timonin.ma...@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.scherbak...@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.mashen...@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.ma...@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.ma...@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 > > >
