From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladislav Shpilevoy Subject: [PATCH v2 2/2] vinyl: in a task use copy of index key defs to protect from alter Date: Thu, 15 Mar 2018 15:19:16 +0300 Message-Id: <451de0337d47f2ebe21f81126fb3e24c57cfbd2b.1521116161.git.v.shpilevoy@tarantool.org> In-Reply-To: References: In-Reply-To: References: To: tarantool-patches@freelists.org Cc: vdavydov.dev@gmail.com, Vladislav Shpilevoy List-ID: A vinyl space can be altered in such a way, that key definitions of indexes are not changed, but comparators do. It is because space format reset can make some indexed fields optional. To be able update key definitions in place, they must not be used in a worker thread. So lets copy key_defs for a worker, and update index key definitions in place. An alternative is key_defs reference counting, but there is open questions what to do in key_defs in mems, ranges, iterators, runs, slices. Now lets do a hotfix of a crash, and then refactoring. Closes #3229 Signed-off-by: Vladislav Shpilevoy --- src/box/vinyl.c | 6 ++++++ src/box/vy_scheduler.c | 25 ++++++++++++++++++++++--- test/engine/ddl.result | 2 +- test/engine/ddl.test.lua | 2 +- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/box/vinyl.c b/src/box/vinyl.c index 225698eb0..229232060 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -1134,6 +1134,8 @@ vinyl_space_commit_alter(struct space *old_space, struct space *new_space) pk->mem_format = new_format; tuple_format_ref(new_format); vy_index_validate_formats(pk); + key_def_update_optionality(pk->key_def, new_format->min_field_count); + key_def_update_optionality(pk->cmp_def, new_format->min_field_count); for (uint32_t i = 1; i < new_space->index_count; ++i) { struct vy_index *index = vy_index(new_space->index[i]); @@ -1153,6 +1155,10 @@ vinyl_space_commit_alter(struct space *old_space, struct space *new_space) tuple_format_ref(index->mem_format_with_colmask); tuple_format_ref(index->mem_format); tuple_format_ref(index->upsert_format); + key_def_update_optionality(index->key_def, + new_format->min_field_count); + key_def_update_optionality(index->cmp_def, + new_format->min_field_count); vy_index_validate_formats(index); } diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c index 382cf0718..b6225ace6 100644 --- a/src/box/vy_scheduler.c +++ b/src/box/vy_scheduler.c @@ -110,6 +110,12 @@ struct vy_task { struct diag diag; /** Index this task is for. */ struct vy_index *index; + /** + * Copies of index->key/cmp_def to protect from + * multithread read/write on alter. + */ + struct key_def *cmp_def; + struct key_def *key_def; /** Range to compact. */ struct vy_range *range; /** Run written by this task. */ @@ -164,6 +170,17 @@ vy_task_new(struct mempool *pool, struct vy_index *index, memset(task, 0, sizeof(*task)); task->ops = ops; task->index = index; + task->cmp_def = key_def_dup(index->cmp_def); + if (task->cmp_def == NULL) { + mempool_free(pool, task); + return NULL; + } + task->key_def = key_def_dup(index->key_def); + if (task->key_def == NULL) { + key_def_delete(task->cmp_def); + mempool_free(pool, task); + return NULL; + } vy_index_ref(index); diag_create(&task->diag); return task; @@ -173,6 +190,8 @@ vy_task_new(struct mempool *pool, struct vy_index *index, static void vy_task_delete(struct mempool *pool, struct vy_task *task) { + key_def_delete(task->cmp_def); + key_def_delete(task->key_def); vy_index_unref(task->index); diag_destroy(&task->diag); TRASH(task); @@ -638,7 +657,7 @@ vy_task_write_run(struct vy_scheduler *scheduler, struct vy_task *task) struct vy_run_writer writer; if (vy_run_writer_create(&writer, task->new_run, index->env->path, index->space_id, index->id, - index->cmp_def, index->key_def, + task->cmp_def, task->key_def, task->page_size, task->bloom_fpr, task->max_output_count) != 0) goto fail; @@ -989,7 +1008,7 @@ vy_task_dump_new(struct vy_scheduler *scheduler, struct vy_index *index, struct vy_stmt_stream *wi; bool is_last_level = (index->run_count == 0); - wi = vy_write_iterator_new(index->cmp_def, index->disk_format, + wi = vy_write_iterator_new(task->cmp_def, index->disk_format, index->upsert_format, index->id == 0, is_last_level, scheduler->read_views); if (wi == NULL) @@ -1264,7 +1283,7 @@ vy_task_compact_new(struct vy_scheduler *scheduler, struct vy_index *index, struct vy_stmt_stream *wi; bool is_last_level = (range->compact_priority == range->slice_count); - wi = vy_write_iterator_new(index->cmp_def, index->disk_format, + wi = vy_write_iterator_new(task->cmp_def, index->disk_format, index->upsert_format, index->id == 0, is_last_level, scheduler->read_views); if (wi == NULL) diff --git a/test/engine/ddl.result b/test/engine/ddl.result index 906c9d11c..c3cb1efc2 100644 --- a/test/engine/ddl.result +++ b/test/engine/ddl.result @@ -394,7 +394,7 @@ s:drop() box.cfg{} --- ... -s = box.schema.create_space('test', {engine = 'memtx'}) +s = box.schema.create_space('test', {engine = engine}) --- ... format = {} diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua index 3b4cd1e5e..25381a3c8 100644 --- a/test/engine/ddl.test.lua +++ b/test/engine/ddl.test.lua @@ -142,7 +142,7 @@ s:drop() -- not only when indexes are updated. -- box.cfg{} -s = box.schema.create_space('test', {engine = 'memtx'}) +s = box.schema.create_space('test', {engine = engine}) format = {} format[1] = {'field1', 'unsigned'} format[2] = {'field2', 'unsigned', is_nullable = true} -- 2.14.3 (Apple Git-98)