[PATCH v2 2/2] vinyl: in a task use copy of index key defs to protect from alter

[Vladislav Shpilevoy]

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 <v.shpilevoy at tarantool.org>
---
 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)