From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 6 Apr 2018 13:59:45 +0300 From: Vladimir Davydov Subject: Re: [PATCH 5/5] alter: call space_vtab::commit_alter after WAL write Message-ID: <20180406105945.us64wqk7auw7mtug@esperanza> References: <743af45528804b8d61e03bc5a8415015f713f26a.1522775293.git.vdavydov.dev@gmail.com> <20180405203728.GB18946@atlas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180405203728.GB18946@atlas> To: Konstantin Osipov Cc: tarantool-patches@freelists.org List-ID: On Thu, Apr 05, 2018 at 11:37:28PM +0300, Konstantin Osipov wrote: > * Vladimir Davydov [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).