Re: [PATCH 02/10] ddl: synchronize user cache with actual data state

[Konstantin Osipov <kostja@tarantool.org>]

* Vladimir Davydov <vdavydov.dev@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.

The yielding transaction kicks-in and does some work, seeing the
dirty state. But doesn't commit yet.

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.

-- 
Konstantin Osipov, Moscow, Russia