Re: [tarantool-patches] [PATCH 3/4] vinyl: abort affected transactions when space is removed from cache

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

[back to the list]

On Mon, Mar 25, 2019 at 11:57:07AM +0300, Георгий Кириченко wrote:
> On Monday, March 25, 2019 11:17:43 AM MSK you wrote:
> > On Mon, Mar 25, 2019 at 08:26:25AM +0300, Георгий Кириченко wrote:
> > > I definitely dislike introducing a new callback, there are already a lot
> > > of
> > > dependencies. Also I'm afraid that this could prevent us from the
> > > transactional ddl implementation.
> > 
> > I don't see a way to fix this issue without introducing a new callback.
> > And frankly I don't quite understand your concern. If the new callback
> > proves to be useless / harmful for transaction DDL, we will remove it -
> > after all engine API isn't carved in stone.
> But leads to increase in amount of work we should be done to implement in 
> future. And we already have a space destroy callback.
> > 
> > > We have a space vtab and destroy, why shouldn't we use them?
> > 
> > Because the space becomes invalid before it gets destroyed - ALTER moves
> > indexes to the new space before writing to WAL.
> So it might be a high time to change it.
> 

It's feasible, I think. However, It'd blow the scope of this particular
patch set as it would most likely require moving reference counting from
vinyl to the upper level. Besides, I don't quite like mixing two
separate functions (space deletion and transaction abortion) in one
callback when we can easily add a new one with a clear semantic. I don't
see anything wrong about removing this callback in future, when
transactional DDL is finally ready. In fact, I've added and removed
ALTER callbacks multiple times and it never posed a problem to me.
Up to Kostja.