[PATCH 1/3] vinyl: use update_def index method to update vy_lsm on ddl

[Vladimir Davydov]

When an index definition is modified by DDL in such a way that doesn't
require index rebuild (e.g. a key part type is changed from unsigned to
integer), we move the index from the old space container to the new one,
see ModifyIndex. In case of Vinyl we also need to update key definitions
and options stored in vy_lsm. We do that in swap_index space method, but
this isn't entirely correct, as this method is also called when an index
is moved without any modifications, see MoveIndex. Let's do this from
update_def index method instead, which seems to be more suitable.

The only reason this code lives in swap_index space method now is that
we didn't have update_def index method when this functionality was
introduced in the first place.
---
 src/box/key_def.c | 56 ++++++++++++++++++++++++++++-------------------
 src/box/key_def.h |  4 ++--
 src/box/vinyl.c   | 14 ++++++++----
 3 files changed, 46 insertions(+), 28 deletions(-)

diff --git a/src/box/key_def.c b/src/box/key_def.c
index ee758eef..9fa1ca49 100644
--- a/src/box/key_def.c
+++ b/src/box/key_def.c
@@ -81,18 +81,26 @@ const struct opt_def part_def_reg[] = {
 	OPT_END,
 };
 
-struct key_def *
-key_def_dup(const struct key_def *src)
+/**
+ * Return the size of memory occupied by the given key definition.
+ */
+static inline size_t
+key_def_copy_size(const struct key_def *def)
 {
 	size_t sz = 0;
-	for (uint32_t i = 0; i < src->part_count; i++)
-		sz += src->parts[i].path_len;
-	sz = key_def_sizeof(src->part_count, sz);
-	struct key_def *res = (struct key_def *)malloc(sz);
-	if (res == NULL) {
-		diag_set(OutOfMemory, sz, "malloc", "res");
-		return NULL;
-	}
+	for (uint32_t i = 0; i < def->part_count; i++)
+		sz += def->parts[i].path_len;
+	return key_def_sizeof(def->part_count, sz);
+}
+
+/**
+ * A helper function for key_def_copy() and key_def_dup() that
+ * copies key definition src of size sz to res without checking
+ * that the two key definitions have the same allocation size.
+ */
+static struct key_def *
+key_def_do_copy(struct key_def *res, const struct key_def *src, size_t sz)
+{
 	memcpy(res, src, sz);
 	/*
 	 * Update the paths pointers so that they refer to the
@@ -112,20 +120,24 @@ key_def_dup(const struct key_def *src)
 }
 
 void
-key_def_swap(struct key_def *old_def, struct key_def *new_def)
+key_def_copy(struct key_def *dest, const struct key_def *src)
 {
-	assert(old_def->part_count == new_def->part_count);
-	for (uint32_t i = 0; i < new_def->part_count; i++) {
-		SWAP(old_def->parts[i], new_def->parts[i]);
-		/*
-		 * Paths are allocated as a part of key_def so
-		 * we need to swap path pointers back - it's OK
-		 * as paths aren't supposed to change.
-		 */
-		assert(old_def->parts[i].path_len == new_def->parts[i].path_len);
-		SWAP(old_def->parts[i].path, new_def->parts[i].path);
+	size_t sz = key_def_copy_size(src);
+	assert(sz = key_def_copy_size(dest));
+	key_def_do_copy(dest, src, sz);
+}
+
+struct key_def *
+key_def_dup(const struct key_def *src)
+{
+	size_t sz = key_def_copy_size(src);
+	struct key_def *res = malloc(sz);
+	if (res == NULL) {
+		diag_set(OutOfMemory, sz, "malloc", "res");
+		return NULL;
 	}
-	SWAP(*old_def, *new_def);
+	key_def_do_copy(res, src, sz);
+	return res;
 }
 
 void
diff --git a/src/box/key_def.h b/src/box/key_def.h
index df83d055..73aefb9a 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -250,11 +250,11 @@ struct key_def *
 key_def_dup(const struct key_def *src);
 
 /**
- * Swap content of two key definitions in memory.
+ * Copy content of key definition src to dest.
  * The two key definitions must have the same size.
  */
 void
-key_def_swap(struct key_def *old_def, struct key_def *new_def);
+key_def_copy(struct key_def *dest, const struct key_def *src);
 
 /**
  * Delete @a key_def.
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index cf7af26b..4ac54138 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -963,6 +963,15 @@ vinyl_index_commit_drop(struct index *index, int64_t lsn)
 	vy_log_tx_try_commit();
 }
 
+static void
+vinyl_index_update_def(struct index *index)
+{
+	struct vy_lsm *lsm = vy_lsm(index);
+	lsm->opts = index->def->opts;
+	key_def_copy(lsm->key_def, index->def->key_def);
+	key_def_copy(lsm->cmp_def, index->def->cmp_def);
+}
+
 static bool
 vinyl_index_depends_on_pk(struct index *index)
 {
@@ -1200,9 +1209,6 @@ vinyl_space_swap_index(struct space *old_space, struct space *new_space,
 	SWAP(old_lsm->check_is_unique, new_lsm->check_is_unique);
 	SWAP(old_lsm->mem_format, new_lsm->mem_format);
 	SWAP(old_lsm->disk_format, new_lsm->disk_format);
-	SWAP(old_lsm->opts, new_lsm->opts);
-	key_def_swap(old_lsm->key_def, new_lsm->key_def);
-	key_def_swap(old_lsm->cmp_def, new_lsm->cmp_def);
 
 	/* Update pointer to the primary key. */
 	vy_lsm_update_pk(old_lsm, vy_lsm(old_space->index_map[0]));
@@ -4741,7 +4747,7 @@ static const struct index_vtab vinyl_index_vtab = {
 	/* .abort_create = */ vinyl_index_abort_create,
 	/* .commit_modify = */ vinyl_index_commit_modify,
 	/* .commit_drop = */ vinyl_index_commit_drop,
-	/* .update_def = */ generic_index_update_def,
+	/* .update_def = */ vinyl_index_update_def,
 	/* .depends_on_pk = */ vinyl_index_depends_on_pk,
 	/* .def_change_requires_rebuild = */
 		vinyl_index_def_change_requires_rebuild,
-- 
2.20.1