Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: Konstantin Osipov <kostja@tarantool.org>
Cc: tarantool-patches@freelists.org
Subject: Re: [PATCH 5/5] alter: call space_vtab::commit_alter after WAL write
Date: Fri, 6 Apr 2018 13:59:45 +0300	[thread overview]
Message-ID: <20180406105945.us64wqk7auw7mtug@esperanza> (raw)
In-Reply-To: <20180405203728.GB18946@atlas>

On Thu, Apr 05, 2018 at 11:37:28PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [18/04/04 08:18]:
> >  void
> >  DropIndex::alter(struct alter_space *alter)
> >  {
> > -	/*
> > -	 * If it's not the primary key, nothing to do --
> > -	 * the dropped index didn't exist in the new space
> > -	 * definition, so does not exist in the created space.
> > -	 */
> > -	if (space_index(alter->new_space, 0) != NULL)
> > -		return;
> > -	/*
> > -	 * OK to drop the primary key. Inform the engine about it,
> > -	 * since it may have to reset handler->replace function,
> > -	 * so that:
> > -	 * - DML returns proper errors rather than crashes the
> > -	 *   program
> > -	 * - when a new primary key is finally added, the space
> > -	 *   can be put back online properly.
> > -	 */
> > -	space_drop_primary_key(alter->new_space);
> > +	if (old_index_def->iid == 0)
> > +		space_drop_primary_key(alter->new_space);
> >  }
> 
> Why did you drop the comments?

Because it's pertinent to memtx only.

> 
> > @@ -736,6 +736,8 @@ memtx_space_drop_primary_key(struct space *space)
> >  {
> >  	struct memtx_space *memtx_space = (struct memtx_space *)space;
> >  	memtx_space->replace = memtx_space_replace_no_keys;
> > +	memtx_space->bsize = 0;
> > +	memtx_space->version++;
> >  }
> 
> Why don't you call it truncate_count?-))

No particular reason. I've nothing against truncate_count.

> I haven't grown accustomed to the approach, can't we use a
> different one?-)

Guess, we need to discuss it f2f.

In short, I don't want to introduce yet another callback for handling
space truncation - it seems natural to do it in space's commit_alter.
And in commit_alter, we need a way to differentiate space truncation vs
rebuild of the primary key. We can't use space bsize for this, as we
currently do, because it is racy to check it after WAL write (there may
have been new insertions into the space after truncation, while we were
waiting for WAL).

      reply	other threads:[~2018-04-06 10:59 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-03 17:37 [PATCH 0/5] alter: fix WAL error handling Vladimir Davydov
2018-04-03 17:37 ` [PATCH 1/5] memtx: rtree: remove pointless index_vtab::begin_build implementation Vladimir Davydov
2018-04-05 20:25   ` Konstantin Osipov
2018-04-03 17:37 ` [PATCH 2/5] memtx: don't call begin_buid and end_build for new pk after recovery Vladimir Davydov
2018-04-03 17:37 ` [PATCH 3/5] vinyl: use disk_format in vy_run_rebuild_index Vladimir Davydov
2018-04-05 20:25   ` Konstantin Osipov
2018-04-03 17:37 ` [PATCH 4/5] vinyl: do not use space_vtab::commit_alter for preparing new indexes Vladimir Davydov
2018-04-03 17:37 ` [PATCH 5/5] alter: call space_vtab::commit_alter after WAL write Vladimir Davydov
2018-04-05 20:37   ` Konstantin Osipov
2018-04-06 10:59     ` Vladimir Davydov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180406105945.us64wqk7auw7mtug@esperanza \
    --to=vdavydov.dev@gmail.com \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [PATCH 5/5] alter: call space_vtab::commit_alter after WAL write' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox