From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 21 Mar 2019 23:16:53 +0300 From: Vladimir Davydov Subject: Re: [tarantool-patches] [PATCH 1/3] Abort vinyl index creation in case of truncation rollback Message-ID: <20190321201653.va4tg4lsqtrjzqx6@esperanza> References: <9e71736be4cc6add4fc856ae8756fcc24b04f4e6.1553112720.git.georgy@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9e71736be4cc6add4fc856ae8756fcc24b04f4e6.1553112720.git.georgy@tarantool.org> To: Georgy Kirichenko Cc: tarantool-patches@freelists.org List-ID: On Wed, Mar 20, 2019 at 11:31:33PM +0300, Georgy Kirichenko wrote: > Abort index create if truncate couldn't be finished because of rollback > or error. Without this vinyl will fail because of internal scheduler > assertion. > > Needed for: 2798 > --- > src/box/alter.cc | 8 ++++++++ > test/engine/errinj.result | 34 ++++++++++++++++++++++++++++++++++ > test/engine/errinj.test.lua | 12 ++++++++++++ > test/engine/suite.ini | 1 + > 4 files changed, 55 insertions(+) > create mode 100644 test/engine/errinj.result > create mode 100644 test/engine/errinj.test.lua > > diff --git a/src/box/alter.cc b/src/box/alter.cc > index 431da12da..84e74ee89 100644 > --- a/src/box/alter.cc > +++ b/src/box/alter.cc > @@ -1312,6 +1312,7 @@ public: > uint32_t iid; > virtual void prepare(struct alter_space *alter); > virtual void commit(struct alter_space *alter, int64_t signature); > + virtual void rollback(struct alter_space *alter); > }; > > void > @@ -1349,6 +1350,13 @@ TruncateIndex::commit(struct alter_space *alter, int64_t signature) > index_commit_create(new_index, signature); > } > > +void > +TruncateIndex::rollback(struct alter_space *alter) > +{ > + struct index *new_index = space_index(alter->new_space, iid); > + index_abort_create(new_index); > +} > + Strictly speaking, it isn't quite correct to do this from 'rollback' callback, because the latter is called only if alter_space_do() succeeded, which may not be the case. I think you better do this from TruncateIndex destructor. Take a look at CreateIndex and RebuildIndex for the reference. > /** > * UpdateSchemaVersion - increment schema_version. Used on > * in alter_space_do(), i.e. when creating or dropping > diff --git a/test/engine/errinj.result b/test/engine/errinj.result > new file mode 100644 > index 000000000..37fc280b9 > --- /dev/null > +++ b/test/engine/errinj.result > @@ -0,0 +1,34 @@ > +test_run = require('test_run') > +--- > +... > +inspector = test_run.new() > +--- > +... > +engine = inspector:get_cfg('engine') > +--- > +... > +errinj = box.error.injection > +--- > +... > +-- test truncate rollback does not abort I can't parse this comment. Let's rephrase it: -- -- Check that on WAL write error space.truncate is rolled back properly. -- > +s = box.schema.space.create('truncate_rollback', {engine = engine}) > +--- > +... > +_ = s:create_index('pk') Please create a secondary index too. Also, insert a few rows into the index and check that they are still there after failed s.truncate. > +--- > +... > +errinj.set('ERRINJ_WAL_IO', true) > +--- > +- ok > +... > +s:truncate() > +--- > +- error: Failed to write to disk > +... > +errinj.set('ERRINJ_WAL_IO', false) > +--- > +- ok > +... > +s:drop() > +--- > +...