Re: [PATCH 1/5] vinyl: don't sync WAL on space alter if not necessary

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

On Mon, Jul 08, 2019 at 12:29:05PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [19/07/05 23:27]:
> > Changes done to an altered space while a new index is being built or
> > the format is being checked are propagated via an on_replace trigger.
> > The problem is there may be transactions that started before the alter
> > request. Their working set can't be checked so we simply abort them.
> > We can't abort transactions that have reached WAL so we also call
> > wal_sync() to flush all pending WAL requests. This is a yielding
> > operation and we call it even if there's no transactions that need
> > to be flushed. As a result, vinyl space alter yields unconditionally,
> > even if the space is empty and there is no pending transactions
> > affecting it. This prevents us from implementing transactional DDL.
> > Let's call wal_sync() only if there's actually at least one pending
> > transaction affecting the altered space and waiting for WAL.
> > ---
> >  src/box/vinyl.c | 51 ++++++++++++++++++++++++++++++---------------------
> >  src/box/vy_tx.c | 11 ++++++++++-
> >  src/box/vy_tx.h |  7 ++++++-
> >  3 files changed, 46 insertions(+), 23 deletions(-)
> > 
> > diff --git a/src/box/vinyl.c b/src/box/vinyl.c
> > index 128b1199..e0de65d0 100644
> > --- a/src/box/vinyl.c
> > +++ b/src/box/vinyl.c
> > @@ -1052,25 +1052,8 @@ vinyl_space_invalidate(struct space *space)
> >  	 * soon as it's done reading disk, which will make the DML
> >  	 * request bail out early, without dereferencing the space.
> >  	 */
> > -	tx_manager_abort_writers_for_ddl(env->xm, space);
> > -}
> 
> Please add a comment for vinyl_space_invalidate, explaining what
> it is used for and who's using it, just like in the comment that
> you removed:

I didn't remove any comments. I just moved a comment from
vy_abort_writers_for_ddl() to vinyl_space_build_index() and
vy_space_check_format().

Regarding vinyl_space_invalidate - I didn't touch it. And it seems to be
pretty well commented as it is, see comments to vinyl_space_invalidate()
and space_vtab.