[PATCH 02/10] ddl: synchronize user cache with actual data state
Vladimir Davydov
vdavydov.dev at gmail.com
Wed Jul 3 23:00:32 MSK 2019
On Wed, Jul 03, 2019 at 10:43:22PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev at gmail.com> [19/07/03 22:34]:
> > To implement transactional DDL, we must make sure that in-memory schema
> > is updated synchronously with system space updates, i.e. on_replace, not
> > on_commit.
> >
> > See also commit 22bedebe715c ("ddl: synchronize privileges cache with
> > actual data state").
>
> Here's a problem with replacing before commit:
>
> Imagine you have a yielding transaction, which is in progress.
>
> You replace state in the metadata cache, it doesn't matter what
> cache - user, view, space, trigger, or whatever.
>
> Then you yield on DDL.
If we yield from a DDL transaction, we must invoke on_rollback triggers
immediately. I don't see any problem with that. If we don't do that now,
I'll change that soon enough.
>
> The yielding transaction kicks-in and does some work, seeing the
> dirty state. But doesn't commit yet.
If on_rollback triggers are run right upon a yield, nobody will see
a dirty state.
>
> Then you roll back.
>
> Your yielding transaction isolation is violated.
>
> Despite this gap I think we should proceed along this path. But we
> also need to make sure all caches are multi-versioned: when you
> replace a value in the cache, only a new transaction should see
> the new value, the old one shouldn't.
>
> This looks fairly straightforward to implement, by adding a
> schema_version to each cacheable schema object and adding
> schema_version to struct txn as well.
>
> Now that we have killed implicit transaction start and separated
> txn from fiber, we can start txn whenever we want and assign it a
> schema version.
>
> So the patch is basically LGTM, but please be aware that moving
> the changes in the cache from on_commit to on_replace breaks
> yielding transaction isolation.
>
> I just asked myself, why did I do it this way in the first place,
> and recalled the above argument.
More information about the Tarantool-patches
mailing list