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

[Vladimir Davydov <vdavydov.dev@gmail.com>]

On Wed, Jul 03, 2019 at 10:43:22PM +0300, Konstantin Osipov wrote:
> * 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.

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.