From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <kostja@tarantool.org>
Date: Thu, 5 Apr 2018 23:37:28 +0300
From: Konstantin Osipov <kostja@tarantool.org>
Subject: Re: [PATCH 5/5] alter: call space_vtab::commit_alter after WAL write
Message-ID: <20180405203728.GB18946@atlas>
References: <cover.1522775293.git.vdavydov.dev@gmail.com>
 <743af45528804b8d61e03bc5a8415015f713f26a.1522775293.git.vdavydov.dev@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <743af45528804b8d61e03bc5a8415015f713f26a.1522775293.git.vdavydov.dev@gmail.com>
To: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

* 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?

> @@ -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?-))
I haven't grown accustomed to the approach, can't we use a
different one?-)

> +--
> +-- gh-3289: drop/truncate leaves the space in inconsistent
> +-- state if WAL write fails.
> +--
> +s = box.schema.space.create('test')
> +---
> +...
> +_ = s:create_index('pk')
> +---
> +...
> +for i = 1, 10 do s:replace{i} end
> +---
> +...
> +errinj.set('ERRINJ_WAL_IO', true)
> +---
> +- ok
> +...
> +s:drop()
> +---
> +- error: Failed to write to disk
> +...
> +s:truncate()
> +---
> +- error: Failed to write to disk
> +...
> +s:drop()
> +---
> +- error: Failed to write to disk
> +...
> +s:truncate()
> +---
> +- error: Failed to write to disk
> +...
> +errinj.set('ERRINJ_WAL_IO', false)
> +---
> +- ok
> +...
> +for i = 1, 10 do s:replace{i + 10} end
> +---
> +...
> +s:select()
> +---
> +- - [1]
> +  - [2]
> +  - [3]
> +  - [4]
> +  - [5]
> +  - [6]
> +  - [7]
> +  - [8]
> +  - [9]
> +  - [10]
> +  - [11]
> +  - [12]
> +  - [13]
> +  - [14]
> +  - [15]
> +  - [16]
> +  - [17]
> +  - [18]
> +  - [19]
> +  - [20]
> +...
> +s:drop()
> +---
> +...
> diff --git a/test/box/errinj.test.lua b/test/box/errinj.test.lua
> index d97cd81f..c12f61a4 100644
> --- a/test/box/errinj.test.lua
> +++ b/test/box/errinj.test.lua
> @@ -366,3 +366,21 @@ errinj.set("ERRINJ_IPROTO_TX_DELAY", false)
>  
>  s:drop()
>  box.schema.user.revoke('guest', 'read,write,execute','universe')
> +
> +--
> +-- gh-3289: drop/truncate leaves the space in inconsistent
> +-- state if WAL write fails.
> +--
> +s = box.schema.space.create('test')
> +_ = s:create_index('pk')
> +for i = 1, 10 do s:replace{i} end
> +errinj.set('ERRINJ_WAL_IO', true)
> +s:drop()
> +s:truncate()
> +s:drop()
> +s:truncate()
> +errinj.set('ERRINJ_WAL_IO', false)
> +for i = 1, 10 do s:replace{i + 10} end
> +s:select()
> +
> +s:drop()

Thank you for a test case.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov