From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Kirill Shcherbatov Subject: [PATCH v1 1/1] box: make functional index creation transactional Date: Fri, 2 Aug 2019 16:05:06 +0300 Message-Id: <1056e3dd6be0f9d03c6feff3651d41884cbd91c6.1564751087.git.kshcherbatov@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit To: tarantool-patches@freelists.org, vdavydov.dev@gmail.com Cc: Kirill Shcherbatov List-ID: The _func_index space trigger used to reject an insertion of a tuple that defines an invalid functional index. As insertion in _index space had been completed before, a garbage is kept in _index space in such case. We need to do something with the yelding _func_index trigger(that rebuilds an index) to wrap all index creation operation in DDL transaction (because only the first DDL operation may yeld now). This problem could be trivially solved with preparatory initialization of index_def function ponter: the memtx_tree index construction would perform all required job in such case. Therefore the following index rebuild in _func_index trigger becomes redundant and should be omitted. In other words, a trivial prefetch operation makes possible a transactional index creation (with space:create_index operation). As for index construction during recovery (a lack of function cache during recovery was the main motivation to introduce _func_index space), it's workflow is kept unchanged. Follow up #1250 Closes #4401 --- src/box/index_def.h | 12 +++++++++ src/box/alter.cc | 46 +++++++++++++++++++++++++++------ src/box/lua/schema.lua | 40 ++++++++++++++++++---------- test/engine/func_index.result | 33 ++++++++++++++++++++--- test/engine/func_index.test.lua | 14 +++++++--- 5 files changed, 116 insertions(+), 29 deletions(-) diff --git a/src/box/index_def.h b/src/box/index_def.h index 62578bd60..d928b23c7 100644 --- a/src/box/index_def.h +++ b/src/box/index_def.h @@ -321,6 +321,18 @@ index_def_set_func(struct index_def *def, struct func *func) def->cmp_def->func_index_func = NULL; } +/** + * Get a func pointer by index definition. + * @param def Index def, containing key definitions. + * @returns not NULL function pointer when index definition + * refers to function and NULL otherwise. + */ +static inline struct func * +index_def_get_func(struct index_def *def) +{ + return def->key_def->func_index_func; +} + /** * Add an index definition to a list, preserving the * first position of the primary key. diff --git a/src/box/alter.cc b/src/box/alter.cc index 92f1d5b22..b92ed056f 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -231,6 +231,23 @@ index_opts_decode(struct index_opts *opts, const char *map, } } +/** + * Helper routine for functional index function verification: + * only a deterministic persistent Lua function may be used in + * functional index for now. + */ +static void +func_index_check_func(struct func *func) { + assert(func != NULL); + if (func->def->language != FUNC_LANGUAGE_LUA || + func->def->body == NULL || !func->def->is_deterministic || + !func->def->is_sandboxed) { + tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS, 0, + "referenced function doesn't satisfy " + "functional index function constraints"); + } +} + /** * Create a index_def object from a record in _index * system space. @@ -285,7 +302,8 @@ index_def_new_from_tuple(struct tuple *tuple, struct space *space) space->def->fields, space->def->field_count, &fiber()->gc) != 0) diag_raise(); - key_def = key_def_new(part_def, part_count, opts.func_id > 0); + bool for_func_index = opts.func_id > 0; + key_def = key_def_new(part_def, part_count, for_func_index); if (key_def == NULL) diag_raise(); struct index_def *index_def = @@ -296,6 +314,16 @@ index_def_new_from_tuple(struct tuple *tuple, struct space *space) auto index_def_guard = make_scoped_guard([=] { index_def_delete(index_def); }); index_def_check_xc(index_def, space_name(space)); space_check_index_def_xc(space, index_def); + /* + * In case of functional index key definition, + * initialize a function pointer, when possible + * (but not during recovery). + */ + struct func *func = NULL; + if (for_func_index && (func = func_by_id(opts.func_id)) != NULL) { + func_index_check_func(func); + index_def_set_func(index_def, func); + } if (index_def->iid == 0 && space->sequence != NULL) index_def_check_sequence(index_def, space->sequence_fieldno, space->sequence_path, @@ -4725,13 +4753,8 @@ on_replace_dd_func_index(struct trigger *trigger, void *event) space = space_cache_find_xc(space_id); index = index_find_xc(space, index_id); func = func_cache_find(fid); - if (func->def->language != FUNC_LANGUAGE_LUA || - func->def->body == NULL || !func->def->is_deterministic || - !func->def->is_sandboxed) { - tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS, 0, - "referenced function doesn't satisfy " - "functional index function constraints"); - } + func_index_check_func(func); + assert(index->def->opts.func_id == func->def->fid); } else if (old_tuple != NULL && new_tuple == NULL) { uint32_t space_id = tuple_field_u32_xc(old_tuple, BOX_FUNC_INDEX_FIELD_SPACE_ID); @@ -4746,6 +4769,13 @@ on_replace_dd_func_index(struct trigger *trigger, void *event) "functional index", "alter"); } + /** + * Index is already initialized for corresponding + * function. Index rebuild is not required. + */ + if (index_def_get_func(index->def) == func) + return; + alter = alter_space_new(space); auto scoped_guard = make_scoped_guard([=] {alter_space_delete(alter);}); alter_space_move_indexes(alter, 0, index->def->iid); diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index 65017d391..89a4d25bc 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -1102,11 +1102,18 @@ box.schema.index.create = function(space_id, name, options) local sequence_proxy = space_sequence_alter_prepare(format, parts, options, space_id, iid, space.name, name) - _index:insert{space_id, iid, name, options.type, index_opts, parts} - space_sequence_alter_commit(sequence_proxy) - if index_opts.func ~= nil then - local _func_index = box.space[box.schema.FUNC_INDEX_ID] - _func_index:insert{space_id, iid, index_opts.func} + local create_f = function () + _index:insert{space_id, iid, name, options.type, index_opts, parts} + space_sequence_alter_commit(sequence_proxy) + if index_opts.func ~= nil then + local _func_index = box.space[box.schema.FUNC_INDEX_ID] + _func_index:insert{space_id, iid, index_opts.func} + end + end + if not box.is_in_txn() then + box.atomic(create_f) + else + create_f() end return space.index[name] end @@ -1114,6 +1121,8 @@ end box.schema.index.drop = function(space_id, index_id) check_param(space_id, 'space_id', 'number') check_param(index_id, 'index_id', 'number') + local _index = box.space[box.schema.INDEX_ID] + local _func_index = box.space[box.schema.FUNC_INDEX_ID] if index_id == 0 then local _space_sequence = box.space[box.schema.SPACE_SEQUENCE_ID] local sequence_tuple = _space_sequence:delete{space_id} @@ -1122,8 +1131,6 @@ box.schema.index.drop = function(space_id, index_id) box.schema.sequence.drop(sequence_tuple.sequence_id) end end - local _index = box.space[box.schema.INDEX_ID] - local _func_index = box.space[box.schema.FUNC_INDEX_ID] for _, v in box.space._func_index:pairs{space_id, index_id} do _func_index:delete({v.space_id, v.index_id}) end @@ -1229,13 +1236,20 @@ box.schema.index.alter = function(space_id, index_id, options) local sequence_proxy = space_sequence_alter_prepare(format, parts, options, space_id, index_id, space.name, options.name) - _index:replace{space_id, index_id, options.name, options.type, - index_opts, parts} - if index_opts.func ~= nil then - local _func_index = box.space[box.schema.FUNC_INDEX_ID] - _func_index:insert{space_id, iid, index_opts.func} + local alter_f = function () + _index:replace{space_id, index_id, options.name, options.type, + index_opts, parts} + if index_opts.func ~= nil then + local _func_index = box.space[box.schema.FUNC_INDEX_ID] + _func_index:insert{space_id, iid, index_opts.func} + end + space_sequence_alter_commit(sequence_proxy) + end + if not box.is_in_txn() then + box.atomic(alter_f) + else + alter_f() end - space_sequence_alter_commit(sequence_proxy) end -- a static box_tuple_t ** instance for calling box_index_* API diff --git a/test/engine/func_index.result b/test/engine/func_index.result index 877b76d5e..fa0d43b36 100644 --- a/test/engine/func_index.result +++ b/test/engine/func_index.result @@ -49,8 +49,13 @@ _ = s:create_index('idx', {func = 6666, parts = {{1, 'unsigned'}}}) | --- | - error: Function '6666' does not exist | ... -s.index.idx:drop() +box.space._index.index.name:count({s.id, 'idx'}) == 0 | --- + | - true + | ... +box.space._func_index:count() == 0 + | --- + | - true | ... -- Can't use non-persistent function in functional index. _ = s:create_index('idx', {func = box.func.s_nonpersistent.id, parts = {{1, 'unsigned'}}}) @@ -58,8 +63,13 @@ _ = s:create_index('idx', {func = box.func.s_nonpersistent.id, parts = {{1, 'uns | - error: 'Wrong index options (field 0): referenced function doesn''t satisfy functional | index function constraints' | ... -s.index.idx:drop() +box.space._index.index.name:count({s.id, 'idx'}) == 0 | --- + | - true + | ... +box.space._func_index:count() == 0 + | --- + | - true | ... -- Can't use non-deterministic function in functional index. _ = s:create_index('idx', {func = box.func.s_ivaliddef1.id, parts = {{1, 'unsigned'}}}) @@ -67,8 +77,13 @@ _ = s:create_index('idx', {func = box.func.s_ivaliddef1.id, parts = {{1, 'unsign | - error: 'Wrong index options (field 0): referenced function doesn''t satisfy functional | index function constraints' | ... -s.index.idx:drop() +box.space._index.index.name:count({s.id, 'idx'}) == 0 | --- + | - true + | ... +box.space._func_index:count() == 0 + | --- + | - true | ... -- Can't use non-sandboxed function in functional index. _ = s:create_index('idx', {func = box.func.s_ivaliddef2.id, parts = {{1, 'unsigned'}}}) @@ -76,8 +91,13 @@ _ = s:create_index('idx', {func = box.func.s_ivaliddef2.id, parts = {{1, 'unsign | - error: 'Wrong index options (field 0): referenced function doesn''t satisfy functional | index function constraints' | ... -s.index.idx:drop() +box.space._index.index.name:count({s.id, 'idx'}) == 0 | --- + | - true + | ... +box.space._func_index:count() == 0 + | --- + | - true | ... -- Can't use non-sequential parts in returned key definition. _ = s:create_index('idx', {func = box.func.ss.id, parts = {{1, 'unsigned'}, {3, 'unsigned'}}}) @@ -731,3 +751,8 @@ box.schema.func.drop('s') box.schema.func.drop('sub') | --- | ... + +box.space._func_index:count() == 0 + | --- + | - true + | ... diff --git a/test/engine/func_index.test.lua b/test/engine/func_index.test.lua index 372ec800d..2878c5b73 100644 --- a/test/engine/func_index.test.lua +++ b/test/engine/func_index.test.lua @@ -19,16 +19,20 @@ _ = s:create_index('idx', {func = box.func.s.id, parts = {{1, 'unsigned'}}}) pk = s:create_index('pk') -- Invalid fid. _ = s:create_index('idx', {func = 6666, parts = {{1, 'unsigned'}}}) -s.index.idx:drop() +box.space._index.index.name:count({s.id, 'idx'}) == 0 +box.space._func_index:count() == 0 -- Can't use non-persistent function in functional index. _ = s:create_index('idx', {func = box.func.s_nonpersistent.id, parts = {{1, 'unsigned'}}}) -s.index.idx:drop() +box.space._index.index.name:count({s.id, 'idx'}) == 0 +box.space._func_index:count() == 0 -- Can't use non-deterministic function in functional index. _ = s:create_index('idx', {func = box.func.s_ivaliddef1.id, parts = {{1, 'unsigned'}}}) -s.index.idx:drop() +box.space._index.index.name:count({s.id, 'idx'}) == 0 +box.space._func_index:count() == 0 -- Can't use non-sandboxed function in functional index. _ = s:create_index('idx', {func = box.func.s_ivaliddef2.id, parts = {{1, 'unsigned'}}}) -s.index.idx:drop() +box.space._index.index.name:count({s.id, 'idx'}) == 0 +box.space._func_index:count() == 0 -- Can't use non-sequential parts in returned key definition. _ = s:create_index('idx', {func = box.func.ss.id, parts = {{1, 'unsigned'}, {3, 'unsigned'}}}) -- Can't use parts started not by 1 field. @@ -248,3 +252,5 @@ idx2:get(3) s:drop() box.schema.func.drop('s') box.schema.func.drop('sub') + +box.space._func_index:count() == 0 -- 2.22.0