Re: [PATCH 12/12] vinyl: allow to modify key definition if it does not require rebuild

[Vladimir Davydov <vdavydov.dev@gmail.com>]

On Sun, Apr 01, 2018 at 12:05:39PM +0300, Vladimir Davydov wrote:
> @@ -945,11 +973,19 @@ vinyl_space_prepare_alter(struct space *old_space, struct space *new_space)
>  	 */
>  	if (env->status != VINYL_ONLINE)
>  		return 0;
> -	/*
> -	 * Regardless of the space emptyness, key definition of an
> -	 * existing index can not be changed, because key
> -	 * definition is already in vylog. See #3169.
> -	 */
> +	/* The space is empty. Allow alter. */
> +	if (pk->stat.disk.count.rows == 0 &&
> +	    pk->stat.memory.count.rows == 0)
> +		return 0;

> +	if (space_def_check_compatibility(old_space->def, new_space->def,
> +					  false) != 0)
> +		return -1;
> +	if (old_space->index_count < new_space->index_count) {
> +		diag_set(ClientError, ER_UNSUPPORTED, "Vinyl",
> +			 "adding an index to a non-empty space");
> +		return -1;
> +	}
> +

These two checks got added by git-revert. They are pointless as we
already have them in vinyl_space_prepare_alter(). I removed them on
the branch. Here's the updated diff:

diff --git a/src/box/key_def.cc b/src/box/key_def.cc
index 2f157419..50786913 100644
--- a/src/box/key_def.cc
+++ b/src/box/key_def.cc
@@ -107,6 +107,15 @@ key_def_dup(const struct key_def *src)
 }
 
 void
+key_def_swap(struct key_def *old_def, struct key_def *new_def)
+{
+	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]);
+	SWAP(*old_def, *new_def);
+}
+
+void
 key_def_delete(struct key_def *def)
 {
 	free(def);
diff --git a/src/box/key_def.h b/src/box/key_def.h
index fe9ee56f..84e3e1e5 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -150,6 +150,13 @@ struct key_def *
 key_def_dup(const struct key_def *src);
 
 /**
+ * Swap content of two key definitions in memory.
+ * The two key definitions must have the same size.
+ */
+void
+key_def_swap(struct key_def *old_def, struct key_def *new_def);
+
+/**
  * Delete @a key_def.
  * @param def Key_def to delete.
  */
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 145fde70..d503704e 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -803,7 +803,7 @@ vinyl_index_commit_create(struct index *index, int64_t lsn)
 		 * the index isn't in the recovery context and we
 		 * need to retry to log it now.
 		 */
-		if (lsm->is_committed) {
+		if (lsm->commit_lsn >= 0) {
 			vy_scheduler_add_lsm(&env->scheduler, lsm);
 			return;
 		}
@@ -828,8 +828,8 @@ vinyl_index_commit_create(struct index *index, int64_t lsn)
 	if (lsm->opts.lsn != 0)
 		lsn = lsm->opts.lsn;
 
-	assert(!lsm->is_committed);
-	lsm->is_committed = true;
+	assert(lsm->commit_lsn < 0);
+	lsm->commit_lsn = lsn;
 
 	assert(lsm->range_count == 1);
 	struct vy_range *range = vy_range_tree_first(lsm->tree);
@@ -854,6 +854,34 @@ vinyl_index_commit_create(struct index *index, int64_t lsn)
 	vy_scheduler_add_lsm(&env->scheduler, lsm);
 }
 
+static void
+vinyl_index_commit_modify(struct index *index, int64_t lsn)
+{
+	struct vy_env *env = vy_env(index->engine);
+	struct vy_lsm *lsm = vy_lsm(index);
+
+	(void)env;
+	assert(env->status == VINYL_ONLINE ||
+	       env->status == VINYL_FINAL_RECOVERY_LOCAL ||
+	       env->status == VINYL_FINAL_RECOVERY_REMOTE);
+
+	if (lsn <= lsm->commit_lsn) {
+		/*
+		 * This must be local recovery from WAL, when
+		 * the operation has already been committed to
+		 * vylog.
+		 */
+		assert(env->status == VINYL_FINAL_RECOVERY_LOCAL);
+		return;
+	}
+
+	lsm->commit_lsn = lsn;
+
+	vy_log_tx_begin();
+	vy_log_modify_lsm(lsm->id, lsm->key_def, lsn);
+	vy_log_tx_try_commit();
+}
+
 /*
  * Delete all runs, ranges, and slices of a given LSM tree
  * from the metadata log.
@@ -945,11 +973,10 @@ vinyl_space_prepare_alter(struct space *old_space, struct space *new_space)
 	 */
 	if (env->status != VINYL_ONLINE)
 		return 0;
-	/*
-	 * Regardless of the space emptyness, key definition of an
-	 * existing index can not be changed, because key
-	 * definition is already in vylog. See #3169.
-	 */
+	/* The space is empty. Allow alter. */
+	if (pk->stat.disk.count.rows == 0 &&
+	    pk->stat.memory.count.rows == 0)
+		return 0;
 	if (old_space->index_count == new_space->index_count) {
 		/* Check index_defs to be unchanged. */
 		for (uint32_t i = 0; i < old_space->index_count; ++i) {
@@ -961,25 +988,14 @@ vinyl_space_prepare_alter(struct space *old_space, struct space *new_space)
 			 * vinyl yet.
 			 */
 			if (index_def_change_requires_rebuild(old_def,
-							      new_def) ||
-			    key_part_cmp(old_def->key_def->parts,
-					 old_def->key_def->part_count,
-					 new_def->key_def->parts,
-					 new_def->key_def->part_count) != 0) {
+							      new_def)) {
 				diag_set(ClientError, ER_UNSUPPORTED, "Vinyl",
-					 "changing the definition of an index");
+					 "changing the definition of "
+					 "a non-empty index");
 				return -1;
 			}
 		}
 	}
-	if (pk->stat.disk.count.rows == 0 &&
-	    pk->stat.memory.count.rows == 0)
-		return 0;
-	/*
-	 * Since space format is not persisted in vylog, it can be
-	 * altered on non-empty space to some state, compatible
-	 * with the old one.
-	 */
 	if (space_def_check_compatibility(old_space->def, new_space->def,
 					  false) != 0)
 		return -1;
@@ -1083,6 +1099,8 @@ vinyl_space_swap_index(struct space *old_space, struct space *new_space,
 	     new_lsm->mem_format_with_colmask);
 	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);
 }
 
 static int
@@ -3931,7 +3949,7 @@ static const struct index_vtab vinyl_index_vtab = {
 	/* .destroy = */ vinyl_index_destroy,
 	/* .commit_create = */ vinyl_index_commit_create,
 	/* .abort_create = */ generic_index_abort_create,
-	/* .commit_modify = */ generic_index_commit_modify,
+	/* .commit_modify = */ vinyl_index_commit_modify,
 	/* .commit_drop = */ vinyl_index_commit_drop,
 	/* .update_def = */ generic_index_update_def,
 	/* .depends_on_pk = */ vinyl_index_depends_on_pk,
diff --git a/src/box/vy_log.c b/src/box/vy_log.c
index 5552065d..172a1b01 100644
--- a/src/box/vy_log.c
+++ b/src/box/vy_log.c
@@ -81,6 +81,7 @@ enum vy_log_key {
 	VY_LOG_KEY_GC_LSN		= 10,
 	VY_LOG_KEY_TRUNCATE_COUNT	= 11,
 	VY_LOG_KEY_CREATE_LSN		= 12,
+	VY_LOG_KEY_MODIFY_LSN		= 13,
 };
 
 /** vy_log_key -> human readable name. */
@@ -98,6 +99,7 @@ static const char *vy_log_key_name[] = {
 	[VY_LOG_KEY_GC_LSN]		= "gc_lsn",
 	[VY_LOG_KEY_TRUNCATE_COUNT]	= "truncate_count",
 	[VY_LOG_KEY_CREATE_LSN]		= "create_lsn",
+	[VY_LOG_KEY_MODIFY_LSN]		= "modify_lsn",
 };
 
 /** vy_log_type -> human readable name. */
@@ -115,6 +117,7 @@ static const char *vy_log_type_name[] = {
 	[VY_LOG_DUMP_LSM]		= "dump_lsm",
 	[VY_LOG_SNAPSHOT]		= "snapshot",
 	[VY_LOG_TRUNCATE_LSM]		= "truncate_lsm",
+	[VY_LOG_MODIFY_LSM]		= "modify_lsm",
 };
 
 /** Metadata log object. */
@@ -251,6 +254,10 @@ vy_log_record_snprint(char *buf, int size, const struct vy_log_record *record)
 		SNPRINT(total, snprintf, buf, size, "%s=%"PRIi64", ",
 			vy_log_key_name[VY_LOG_KEY_CREATE_LSN],
 			record->create_lsn);
+	if (record->modify_lsn > 0)
+		SNPRINT(total, snprintf, buf, size, "%s=%"PRIi64", ",
+			vy_log_key_name[VY_LOG_KEY_MODIFY_LSN],
+			record->modify_lsn);
 	if (record->dump_lsn > 0)
 		SNPRINT(total, snprintf, buf, size, "%s=%"PRIi64", ",
 			vy_log_key_name[VY_LOG_KEY_DUMP_LSN],
@@ -360,6 +367,11 @@ vy_log_record_encode(const struct vy_log_record *record,
 		size += mp_sizeof_uint(record->create_lsn);
 		n_keys++;
 	}
+	if (record->modify_lsn > 0) {
+		size += mp_sizeof_uint(VY_LOG_KEY_MODIFY_LSN);
+		size += mp_sizeof_uint(record->modify_lsn);
+		n_keys++;
+	}
 	if (record->dump_lsn > 0) {
 		size += mp_sizeof_uint(VY_LOG_KEY_DUMP_LSN);
 		size += mp_sizeof_uint(record->dump_lsn);
@@ -432,6 +444,10 @@ vy_log_record_encode(const struct vy_log_record *record,
 		pos = mp_encode_uint(pos, VY_LOG_KEY_CREATE_LSN);
 		pos = mp_encode_uint(pos, record->create_lsn);
 	}
+	if (record->modify_lsn > 0) {
+		pos = mp_encode_uint(pos, VY_LOG_KEY_MODIFY_LSN);
+		pos = mp_encode_uint(pos, record->modify_lsn);
+	}
 	if (record->dump_lsn > 0) {
 		pos = mp_encode_uint(pos, VY_LOG_KEY_DUMP_LSN);
 		pos = mp_encode_uint(pos, record->dump_lsn);
@@ -552,6 +568,9 @@ vy_log_record_decode(struct vy_log_record *record,
 		case VY_LOG_KEY_CREATE_LSN:
 			record->create_lsn = mp_decode_uint(&pos);
 			break;
+		case VY_LOG_KEY_MODIFY_LSN:
+			record->modify_lsn = mp_decode_uint(&pos);
+			break;
 		case VY_LOG_KEY_DUMP_LSN:
 			record->dump_lsn = mp_decode_uint(&pos);
 			break;
@@ -568,7 +587,7 @@ vy_log_record_decode(struct vy_log_record *record,
 			goto fail;
 		}
 	}
-	if (record->type == VY_LOG_CREATE_LSM && record->create_lsn == 0) {
+	if (record->type == VY_LOG_CREATE_LSM) {
 		/*
 		 * We used to use LSN as unique LSM tree identifier
 		 * and didn't store LSN separately so if there's
@@ -577,7 +596,14 @@ vy_log_record_decode(struct vy_log_record *record,
 		 * fact the LSN of the WAL record that committed
 		 * the LSM tree.
 		 */
-		record->create_lsn = record->lsm_id;
+		if (record->create_lsn == 0)
+			record->create_lsn = record->lsm_id;
+		/*
+		 * If the LSM tree has never been modified, initialize
+		 * 'modify_lsn' with 'create_lsn'.
+		 */
+		if (record->modify_lsn == 0)
+			record->modify_lsn = record->create_lsn;
 	}
 	return 0;
 fail:
@@ -1172,7 +1198,7 @@ vy_recovery_create_lsm(struct vy_recovery *recovery, int64_t id,
 		       uint32_t space_id, uint32_t index_id,
 		       const struct key_part_def *key_parts,
 		       uint32_t key_part_count, int64_t create_lsn,
-		       int64_t dump_lsn)
+		       int64_t modify_lsn, int64_t dump_lsn)
 {
 	struct vy_lsm_recovery_info *lsm;
 	struct key_part_def *key_parts_copy;
@@ -1259,6 +1285,7 @@ vy_recovery_create_lsm(struct vy_recovery *recovery, int64_t id,
 	lsm->key_part_count = key_part_count;
 	lsm->is_dropped = false;
 	lsm->create_lsn = create_lsn;
+	lsm->modify_lsn = modify_lsn;
 	lsm->dump_lsn = dump_lsn;
 
 	/*
@@ -1286,6 +1313,43 @@ vy_recovery_create_lsm(struct vy_recovery *recovery, int64_t id,
 }
 
 /**
+ * Handle a VY_LOG_MODIFY_LSM log record.
+ * This function updates key definition of the LSM tree with ID @id.
+ * Return 0 on success, -1 on failure.
+ */
+static int
+vy_recovery_modify_lsm(struct vy_recovery *recovery, int64_t id,
+		       const struct key_part_def *key_parts,
+		       uint32_t key_part_count, int64_t modify_lsn)
+{
+	struct vy_lsm_recovery_info *lsm;
+	lsm = vy_recovery_lookup_lsm(recovery, id);
+	if (lsm == NULL) {
+		diag_set(ClientError, ER_INVALID_VYLOG_FILE,
+			 tt_sprintf("Update of unregistered LSM tree %lld",
+				    (long long)id));
+		return -1;
+	}
+	if (lsm->is_dropped) {
+		diag_set(ClientError, ER_INVALID_VYLOG_FILE,
+			 tt_sprintf("Update of deleted LSM tree %lld",
+				    (long long)id));
+		return -1;
+	}
+	free(lsm->key_parts);
+	lsm->key_parts = malloc(sizeof(*key_parts) * key_part_count);
+	if (lsm->key_parts == NULL) {
+		diag_set(OutOfMemory, sizeof(*key_parts) * key_part_count,
+			 "malloc", "struct key_part_def");
+		return -1;
+	}
+	memcpy(lsm->key_parts, key_parts, sizeof(*key_parts) * key_part_count);
+	lsm->key_part_count = key_part_count;
+	lsm->modify_lsn = modify_lsn;
+	return 0;
+}
+
+/**
  * Handle a VY_LOG_DROP_LSM log record.
  * This function marks the LSM tree with ID @id as dropped.
  * All ranges and runs of the LSM tree must have been deleted by now.
@@ -1757,7 +1821,13 @@ vy_recovery_process_record(struct vy_recovery *recovery,
 		rc = vy_recovery_create_lsm(recovery, record->lsm_id,
 				record->space_id, record->index_id,
 				record->key_parts, record->key_part_count,
-				record->create_lsn, record->dump_lsn);
+				record->create_lsn, record->modify_lsn,
+				record->dump_lsn);
+		break;
+	case VY_LOG_MODIFY_LSM:
+		rc = vy_recovery_modify_lsm(recovery, record->lsm_id,
+				record->key_parts, record->key_part_count,
+				record->modify_lsn);
 		break;
 	case VY_LOG_DROP_LSM:
 		rc = vy_recovery_drop_lsm(recovery, record->lsm_id);
@@ -2007,6 +2077,7 @@ vy_log_append_lsm(struct xlog *xlog, struct vy_lsm_recovery_info *lsm)
 	record.key_parts = lsm->key_parts;
 	record.key_part_count = lsm->key_part_count;
 	record.create_lsn = lsm->create_lsn;
+	record.modify_lsn = lsm->modify_lsn;
 	record.dump_lsn = lsm->dump_lsn;
 	if (vy_log_append_record(xlog, &record) != 0)
 		return -1;
diff --git a/src/box/vy_log.h b/src/box/vy_log.h
index 702963d5..1b2b419f 100644
--- a/src/box/vy_log.h
+++ b/src/box/vy_log.h
@@ -164,6 +164,11 @@ enum vy_log_record_type {
 	 * WAL records written after truncation.
 	 */
 	VY_LOG_TRUNCATE_LSM		= 12,
+	/**
+	 * Modify key definition of an LSM tree.
+	 * Requires vy_log_record::lsm_id, key_def, modify_lsn.
+	 */
+	VY_LOG_MODIFY_LSM		= 13,
 
 	vy_log_record_type_MAX
 };
@@ -202,6 +207,8 @@ struct vy_log_record {
 	uint32_t key_part_count;
 	/** LSN of the WAL row that created the LSM tree. */
 	int64_t create_lsn;
+	/** LSN of the WAL row that last modified the LSM tree. */
+	int64_t modify_lsn;
 	/** Max LSN stored on disk. */
 	int64_t dump_lsn;
 	/**
@@ -255,6 +262,8 @@ struct vy_lsm_recovery_info {
 	bool is_dropped;
 	/** LSN of the WAL row that created the LSM tree. */
 	int64_t create_lsn;
+	/** LSN of the WAL row that last modified the LSM tree. */
+	int64_t modify_lsn;
 	/** LSN of the last LSM tree dump. */
 	int64_t dump_lsn;
 	/**
@@ -516,6 +525,19 @@ vy_log_create_lsm(int64_t id, uint32_t space_id, uint32_t index_id,
 	vy_log_write(&record);
 }
 
+/** Helper to log a vinyl LSM tree modification. */
+static inline void
+vy_log_modify_lsm(int64_t id, const struct key_def *key_def, int64_t modify_lsn)
+{
+	struct vy_log_record record;
+	vy_log_record_init(&record);
+	record.type = VY_LOG_MODIFY_LSM;
+	record.lsm_id = id;
+	record.key_def = key_def;
+	record.modify_lsn = modify_lsn;
+	vy_log_write(&record);
+}
+
 /** Helper to log a vinyl LSM tree drop. */
 static inline void
 vy_log_drop_lsm(int64_t id)
diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c
index af6dde50..a8ed39ad 100644
--- a/src/box/vy_lsm.c
+++ b/src/box/vy_lsm.c
@@ -182,6 +182,7 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env,
 	lsm->id = -1;
 	lsm->refs = 1;
 	lsm->dump_lsn = -1;
+	lsm->commit_lsn = -1;
 	vy_cache_create(&lsm->cache, cache_env, cmp_def);
 	rlist_create(&lsm->sealed);
 	vy_range_tree_new(lsm->tree);
@@ -479,7 +480,7 @@ vy_lsm_recover(struct vy_lsm *lsm, struct vy_recovery *recovery,
 		 bool is_checkpoint_recovery, bool force_recovery)
 {
 	assert(lsm->id < 0);
-	assert(!lsm->is_committed);
+	assert(lsm->commit_lsn < 0);
 	assert(lsm->range_count == 0);
 
 	/*
@@ -535,7 +536,7 @@ vy_lsm_recover(struct vy_lsm *lsm, struct vy_recovery *recovery,
 	}
 
 	lsm->id = lsm_info->id;
-	lsm->is_committed = true;
+	lsm->commit_lsn = lsm_info->modify_lsn;
 
 	if (lsn < lsm_info->create_lsn || lsm_info->is_dropped) {
 		/*
diff --git a/src/box/vy_lsm.h b/src/box/vy_lsm.h
index f5862a52..201b5a56 100644
--- a/src/box/vy_lsm.h
+++ b/src/box/vy_lsm.h
@@ -255,11 +255,13 @@ struct vy_lsm {
 	 * been dumped yet.
 	 */
 	int64_t dump_lsn;
-	/*
-	 * This flag is set if the LSM tree creation was
-	 * committed to the metadata log.
+	/**
+	 * LSN of the WAL row that created or last modified
+	 * this LSM tree. We store it in vylog so that during
+	 * local recovery we can replay vylog records we failed
+	 * to log before restart.
 	 */
-	bool is_committed;
+	int64_t commit_lsn;
 	/**
 	 * This flag is set if the LSM tree was dropped.
 	 * It is also set on local recovery if the LSM tree
diff --git a/test/engine/ddl.result b/test/engine/ddl.result
index c3cb1efc..308aefb0 100644
--- a/test/engine/ddl.result
+++ b/test/engine/ddl.result
@@ -391,9 +391,6 @@ s:drop()
 -- gh-3229: update optionality if a space format is changed too,
 -- not only when indexes are updated.
 --
-box.cfg{}
----
-...
 s = box.schema.create_space('test', {engine = engine})
 ---
 ...
@@ -466,3 +463,113 @@ sk:get({5})
 s:drop()
 ---
 ...
+--
+-- Modify key definition without index rebuild.
+--
+s = box.schema.create_space('test', {engine = engine})
+---
+...
+i1 = s:create_index('i1', {unique = true,  parts = {1, 'unsigned'}})
+---
+...
+i2 = s:create_index('i2', {unique = false, parts = {2, 'unsigned'}})
+---
+...
+i3 = s:create_index('i3', {unique = true,  parts = {3, 'unsigned'}})
+---
+...
+_ = s:insert{1, 2, 3}
+---
+...
+box.snapshot()
+---
+- ok
+...
+_ = s:insert{3, 2, 1}
+---
+...
+i1:alter{parts = {1, 'integer'}}
+---
+...
+_ = s:insert{-1, 2, 2}
+---
+...
+i1:select()
+---
+- - [-1, 2, 2]
+  - [1, 2, 3]
+  - [3, 2, 1]
+...
+i2:select()
+---
+- - [-1, 2, 2]
+  - [1, 2, 3]
+  - [3, 2, 1]
+...
+i3:select()
+---
+- - [3, 2, 1]
+  - [-1, 2, 2]
+  - [1, 2, 3]
+...
+i2:alter{parts = {2, 'integer'}}
+---
+...
+i3:alter{parts = {3, 'integer'}}
+---
+...
+_ = s:replace{-1, -1, -1}
+---
+...
+i1:select()
+---
+- - [-1, -1, -1]
+  - [1, 2, 3]
+  - [3, 2, 1]
+...
+i2:select()
+---
+- - [-1, -1, -1]
+  - [1, 2, 3]
+  - [3, 2, 1]
+...
+i3:select()
+---
+- - [-1, -1, -1]
+  - [3, 2, 1]
+  - [1, 2, 3]
+...
+box.snapshot()
+---
+- ok
+...
+_ = s:replace{-1, -2, -3}
+---
+...
+_ = s:replace{-3, -2, -1}
+---
+...
+i1:select()
+---
+- - [-3, -2, -1]
+  - [-1, -2, -3]
+  - [1, 2, 3]
+  - [3, 2, 1]
+...
+i2:select()
+---
+- - [-3, -2, -1]
+  - [-1, -2, -3]
+  - [1, 2, 3]
+  - [3, 2, 1]
+...
+i3:select()
+---
+- - [-1, -2, -3]
+  - [-3, -2, -1]
+  - [3, 2, 1]
+  - [1, 2, 3]
+...
+s:drop()
+---
+...
diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua
index 25381a3c..019e18a1 100644
--- a/test/engine/ddl.test.lua
+++ b/test/engine/ddl.test.lua
@@ -141,7 +141,6 @@ s:drop()
 -- gh-3229: update optionality if a space format is changed too,
 -- not only when indexes are updated.
 --
-box.cfg{}
 s = box.schema.create_space('test', {engine = engine})
 format = {}
 format[1] = {'field1', 'unsigned'}
@@ -162,3 +161,37 @@ pk:get({4})
 sk:select({box.NULL})
 sk:get({5})
 s:drop()
+
+--
+-- Modify key definition without index rebuild.
+--
+s = box.schema.create_space('test', {engine = engine})
+i1 = s:create_index('i1', {unique = true,  parts = {1, 'unsigned'}})
+i2 = s:create_index('i2', {unique = false, parts = {2, 'unsigned'}})
+i3 = s:create_index('i3', {unique = true,  parts = {3, 'unsigned'}})
+
+_ = s:insert{1, 2, 3}
+box.snapshot()
+_ = s:insert{3, 2, 1}
+
+i1:alter{parts = {1, 'integer'}}
+_ = s:insert{-1, 2, 2}
+i1:select()
+i2:select()
+i3:select()
+
+i2:alter{parts = {2, 'integer'}}
+i3:alter{parts = {3, 'integer'}}
+_ = s:replace{-1, -1, -1}
+i1:select()
+i2:select()
+i3:select()
+
+box.snapshot()
+_ = s:replace{-1, -2, -3}
+_ = s:replace{-3, -2, -1}
+i1:select()
+i2:select()
+i3:select()
+
+s:drop()
diff --git a/test/engine/replica_join.result b/test/engine/replica_join.result
index a003c0d8..39d857fe 100644
--- a/test/engine/replica_join.result
+++ b/test/engine/replica_join.result
@@ -293,6 +293,12 @@ for k = 8, 1, -1 do box.space.test:update(k, {{'-', 2, 8}}) end
 for k = 9, 16 do box.space.test:delete(k) end
 ---
 ...
+box.space.test.index.primary:alter{parts = {1, 'integer'}}
+---
+...
+for k = -8, -1 do box.space.test:insert{k, 9 + k} end
+---
+...
 _ = box.space.test2:upsert({1, 'test1', 11}, {{'+', 3, 1}})
 ---
 ...
@@ -333,7 +339,15 @@ test_run:cmd('switch replica')
 ...
 box.space.test:select()
 ---
-- - [1, 8]
+- - [-8, 1]
+  - [-7, 2]
+  - [-6, 3]
+  - [-5, 4]
+  - [-4, 5]
+  - [-3, 6]
+  - [-2, 7]
+  - [-1, 8]
+  - [1, 8]
   - [2, 7]
   - [3, 6]
   - [4, 5]
@@ -344,13 +358,21 @@ box.space.test:select()
 ...
 box.space.test.index.secondary:select()
 ---
-- - [8, 1]
+- - [-8, 1]
+  - [8, 1]
+  - [-7, 2]
   - [7, 2]
+  - [-6, 3]
   - [6, 3]
+  - [-5, 4]
   - [5, 4]
+  - [-4, 5]
   - [4, 5]
+  - [-3, 6]
   - [3, 6]
+  - [-2, 7]
   - [2, 7]
+  - [-1, 8]
   - [1, 8]
 ...
 box.space.test2:select()
diff --git a/test/engine/replica_join.test.lua b/test/engine/replica_join.test.lua
index 5c6ed11a..1792281e 100644
--- a/test/engine/replica_join.test.lua
+++ b/test/engine/replica_join.test.lua
@@ -72,6 +72,8 @@ _ = test_run:cmd("cleanup server replica")
 -- new data
 for k = 8, 1, -1 do box.space.test:update(k, {{'-', 2, 8}}) end
 for k = 9, 16 do box.space.test:delete(k) end
+box.space.test.index.primary:alter{parts = {1, 'integer'}}
+for k = -8, -1 do box.space.test:insert{k, 9 + k} end
 _ = box.space.test2:upsert({1, 'test1', 11}, {{'+', 3, 1}})
 _ = box.space.test2:update({2, 'test2'}, {{'+', 3, 2}})
 _ = box.space.test2:delete{3, 'test3'}
diff --git a/test/engine/truncate.result b/test/engine/truncate.result
index b6e1a99a..2222804b 100644
--- a/test/engine/truncate.result
+++ b/test/engine/truncate.result
@@ -391,7 +391,7 @@ _ = s3:insert{789, 987}
 ...
 -- Check that index drop, create, and alter called after space
 -- truncate do not break recovery (gh-2615)
-s4 = box.schema.create_space('test4', {engine = 'memtx'})
+s4 = box.schema.create_space('test4', {engine = engine})
 ---
 ...
 _ = s4:create_index('i1', {parts = {1, 'string'}})
diff --git a/test/engine/truncate.test.lua b/test/engine/truncate.test.lua
index 66cbd156..df2797a1 100644
--- a/test/engine/truncate.test.lua
+++ b/test/engine/truncate.test.lua
@@ -175,7 +175,7 @@ s3:truncate()
 _ = s3:insert{789, 987}
 -- Check that index drop, create, and alter called after space
 -- truncate do not break recovery (gh-2615)
-s4 = box.schema.create_space('test4', {engine = 'memtx'})
+s4 = box.schema.create_space('test4', {engine = engine})
 _ = s4:create_index('i1', {parts = {1, 'string'}})
 _ = s4:create_index('i3', {parts = {3, 'string'}})
 _ = s4:insert{'zzz', 111, 'yyy'}
diff --git a/test/vinyl/ddl.result b/test/vinyl/ddl.result
index 6f393c69..456977e5 100644
--- a/test/vinyl/ddl.result
+++ b/test/vinyl/ddl.result
@@ -87,7 +87,7 @@ index2 = space:create_index('secondary', { parts = {2, 'unsigned'} })
 ...
 space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}})
 ---
-- error: Vinyl does not support changing the definition of an index
+- error: Vinyl does not support changing the definition of a non-empty index
 ...
 #box.space._index:select({space.id})
 ---
@@ -116,7 +116,7 @@ index2 = space:create_index('secondary', { parts = {2, 'unsigned'} })
 ...
 space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}})
 ---
-- error: Vinyl does not support changing the definition of an index
+- error: Vinyl does not support changing the definition of a non-empty index
 ...
 #box.space._index:select({space.id})
 ---
@@ -145,7 +145,7 @@ index2 = space:create_index('secondary', { parts = {2, 'unsigned'} })
 ...
 space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}})
 ---
-- error: Vinyl does not support changing the definition of an index
+- error: Vinyl does not support changing the definition of a non-empty index
 ...
 #box.space._index:select({space.id})
 ---
@@ -165,7 +165,7 @@ index2 = space:create_index('secondary', { parts = {2, 'unsigned'} })
 ...
 space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}})
 ---
-- error: Vinyl does not support changing the definition of an index
+- error: Vinyl does not support changing the definition of a non-empty index
 ...
 box.snapshot()
 ---
@@ -174,27 +174,33 @@ box.snapshot()
 while space.index.primary:info().rows ~= 0 do fiber.sleep(0.01) end
 ---
 ...
--- After a dump REPLACE + DELETE = nothing, so the space is empty
--- but an index can not be altered.
+-- after a dump REPLACE + DELETE = nothing, so the space is empty now and
+-- can be altered.
 index2 = space:create_index('secondary', { parts = {2, 'unsigned'} })
 ---
 ...
 space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}})
 ---
-- error: Vinyl does not support changing the definition of an index
 ...
--- Space format still can be altered.
-format = {}
+#box.space._index:select({space.id})
 ---
+- 2
 ...
-format[1] = {name = 'field1', type = 'unsigned'}
+box.space._index:get{space.id, 0}[6]
 ---
+- [[0, 'unsigned'], [1, 'unsigned']]
 ...
-format[2] = {name = 'field2', type = 'unsigned'}
+space:insert({1, 2})
 ---
+- [1, 2]
 ...
-space:format(format)
+index:select{}
+---
+- - [1, 2]
+...
+index2:select{}
 ---
+- - [1, 2]
 ...
 space:drop()
 ---
@@ -230,7 +236,7 @@ index2 = space:create_index('secondary', { parts = {2, 'unsigned'} })
 ...
 space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}})
 ---
-- error: Vinyl does not support changing the definition of an index
+- error: Vinyl does not support changing the definition of a non-empty index
 ...
 -- After compaction the REPLACE + DELETE + DELETE = nothing, so
 -- the space is now empty and can be altered.
@@ -256,10 +262,8 @@ while space.index.primary:info().rows ~= 0 do fiber.sleep(0.01) end
 index2 = space:create_index('secondary', { parts = {2, 'unsigned'} })
 ---
 ...
--- Can not alter an index even if it becames empty after dump.
 space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}})
 ---
-- error: Vinyl does not support changing the definition of an index
 ...
 space:drop()
 ---
@@ -287,7 +291,7 @@ space:auto_increment{3}
 ...
 box.space._index:replace{space.id, 0, 'pk', 'tree', {unique=true}, {{0, 'unsigned'}, {1, 'unsigned'}}}
 ---
-- error: Vinyl does not support changing the definition of an index
+- error: Vinyl does not support changing the definition of a non-empty index
 ...
 space:select{}
 ---
@@ -828,7 +832,7 @@ s.index.secondary.unique
 ...
 s.index.secondary:alter{unique = true} -- error
 ---
-- error: Vinyl does not support changing the definition of an index
+- error: Vinyl does not support changing the definition of a non-empty index
 ...
 s.index.secondary.unique
 ---
@@ -846,43 +850,3 @@ s.index.secondary:select(10)
 s:drop()
 ---
 ...
---
--- gh-3169: vinyl index key definition can not be altered even if
--- the index is empty.
---
-s = box.schema.space.create('vinyl', {engine = 'vinyl'})
----
-...
-i = s:create_index('pk')
----
-...
-i:alter{parts = {1, 'integer'}}
----
-- error: Vinyl does not support changing the definition of an index
-...
-_ = s:replace{-1}
----
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
-...
-_ = s:replace{1}
----
-...
-_ = s:replace{-2}
----
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
-...
-_ = s:replace{3}
----
-...
-_ = s:replace{-3}
----
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
-...
-s:select{}
----
-- - [1]
-  - [3]
-...
-s:drop()
----
-...
diff --git a/test/vinyl/ddl.test.lua b/test/vinyl/ddl.test.lua
index f050799e..8ce8b920 100644
--- a/test/vinyl/ddl.test.lua
+++ b/test/vinyl/ddl.test.lua
@@ -59,15 +59,15 @@ space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}})
 box.snapshot()
 while space.index.primary:info().rows ~= 0 do fiber.sleep(0.01) end
 
--- After a dump REPLACE + DELETE = nothing, so the space is empty
--- but an index can not be altered.
+-- after a dump REPLACE + DELETE = nothing, so the space is empty now and
+-- can be altered.
 index2 = space:create_index('secondary', { parts = {2, 'unsigned'} })
 space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}})
--- Space format still can be altered.
-format = {}
-format[1] = {name = 'field1', type = 'unsigned'}
-format[2] = {name = 'field2', type = 'unsigned'}
-space:format(format)
+#box.space._index:select({space.id})
+box.space._index:get{space.id, 0}[6]
+space:insert({1, 2})
+index:select{}
+index2:select{}
 space:drop()
 
 space = box.schema.space.create('test', { engine = 'vinyl' })
@@ -91,7 +91,6 @@ box.snapshot()
 -- Wait until the dump is finished.
 while space.index.primary:info().rows ~= 0 do fiber.sleep(0.01) end
 index2 = space:create_index('secondary', { parts = {2, 'unsigned'} })
--- Can not alter an index even if it becames empty after dump.
 space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}})
 
 space:drop()
@@ -305,18 +304,3 @@ s.index.secondary.unique
 s:insert{2, 10}
 s.index.secondary:select(10)
 s:drop()
-
---
--- gh-3169: vinyl index key definition can not be altered even if
--- the index is empty.
---
-s = box.schema.space.create('vinyl', {engine = 'vinyl'})
-i = s:create_index('pk')
-i:alter{parts = {1, 'integer'}}
-_ = s:replace{-1}
-_ = s:replace{1}
-_ = s:replace{-2}
-_ = s:replace{3}
-_ = s:replace{-3}
-s:select{}
-s:drop()
diff --git a/test/vinyl/gh.result b/test/vinyl/gh.result
index 32e4b6fb..9c72acc8 100644
--- a/test/vinyl/gh.result
+++ b/test/vinyl/gh.result
@@ -144,7 +144,7 @@ s:insert{5, 5}
 ...
 s.index.primary:alter({parts={2,'unsigned'}})
 ---
-- error: Vinyl does not support changing the definition of an index
+- error: Vinyl does not support changing the definition of a non-empty index
 ...
 s:drop()
 ---
diff --git a/test/vinyl/layout.result b/test/vinyl/layout.result
index 4af2c024..e6294d3c 100644
--- a/test/vinyl/layout.result
+++ b/test/vinyl/layout.result
@@ -20,7 +20,7 @@ space = box.schema.space.create('test', {engine='vinyl'})
 _ = space:create_index('pk', {parts = {{1, 'string', collation = 'unicode'}}, run_count_per_level=3})
 ---
 ...
-_ = space:create_index('sk', {parts = {{2, 'unsigned', is_nullable = true}}, run_count_per_level=3})
+_ = space:create_index('sk', {parts = {{2, 'unsigned'}}, run_count_per_level=3})
 ---
 ...
 -- Empty run
@@ -35,6 +35,9 @@ box.snapshot()
 ---
 - ok
 ...
+space.index.sk:alter{parts = {{2, 'unsigned', is_nullable = true}}}
+---
+...
 space:replace{'ЭЭЭ', box.NULL}
 ---
 - ['ЭЭЭ', null]
@@ -123,16 +126,16 @@ test_run:cmd("push filter 'offset: .*' to 'offset: <offset>'")
 ...
 result
 ---
-- - - 00000000000000000009.vylog
+- - - 00000000000000000010.vylog
     - - HEADER:
           type: INSERT
         BODY:
-          tuple: [0, {7: [{'field': 0, 'collation': 1, 'type': 'string'}], 9: 9, 12: 3,
-              6: 512}]
+          tuple: [0, {6: 512, 7: [{'field': 0, 'collation': 1, 'type': 'string'}],
+              9: 10, 12: 3, 13: 7}]
       - HEADER:
           type: INSERT
         BODY:
-          tuple: [5, {2: 8, 9: 9}]
+          tuple: [5, {2: 8, 9: 10}]
       - HEADER:
           type: INSERT
         BODY:
@@ -153,11 +156,11 @@ result
           type: INSERT
         BODY:
           tuple: [0, {0: 2, 5: 1, 6: 512, 7: [{'field': 1, 'is_nullable': true, 'type': 'unsigned'}],
-              9: 9, 12: 4}]
+              9: 10, 12: 4, 13: 7}]
       - HEADER:
           type: INSERT
         BODY:
-          tuple: [5, {0: 2, 2: 6, 9: 9}]
+          tuple: [5, {0: 2, 2: 6, 9: 10}]
       - HEADER:
           type: INSERT
         BODY:
@@ -197,7 +200,7 @@ result
           timestamp: <timestamp>
           type: INSERT
         BODY:
-          tuple: [5, {0: 2, 2: 10, 9: 12}]
+          tuple: [5, {0: 2, 2: 10, 9: 13}]
       - HEADER:
           timestamp: <timestamp>
           type: INSERT
@@ -207,7 +210,7 @@ result
           timestamp: <timestamp>
           type: INSERT
         BODY:
-          tuple: [10, {0: 2, 9: 12}]
+          tuple: [10, {0: 2, 9: 13}]
       - HEADER:
           timestamp: <timestamp>
           type: INSERT
@@ -217,7 +220,7 @@ result
           timestamp: <timestamp>
           type: INSERT
         BODY:
-          tuple: [5, {2: 12, 9: 12}]
+          tuple: [5, {2: 12, 9: 13}]
       - HEADER:
           timestamp: <timestamp>
           type: INSERT
@@ -227,16 +230,16 @@ result
           timestamp: <timestamp>
           type: INSERT
         BODY:
-          tuple: [10, {9: 12}]
+          tuple: [10, {9: 13}]
   - - 00000000000000000008.index
     - - HEADER:
           type: RUNINFO
         BODY:
-          min_lsn: 7
+          min_lsn: 8
           max_key: ['ЭЭЭ']
           page_count: 1
           bloom_filter: <bloom_filter>
-          max_lsn: 9
+          max_lsn: 10
           min_key: ['ёёё']
       - HEADER:
           type: PAGEINFO
@@ -249,17 +252,17 @@ result
           min_key: ['ёёё']
   - - 00000000000000000008.run
     - - HEADER:
-          lsn: 9
+          lsn: 10
           type: INSERT
         BODY:
           tuple: ['ёёё', null]
       - HEADER:
-          lsn: 8
+          lsn: 9
           type: INSERT
         BODY:
           tuple: ['эээ', null]
       - HEADER:
-          lsn: 7
+          lsn: 8
           type: INSERT
         BODY:
           tuple: ['ЭЭЭ', null]
@@ -271,11 +274,11 @@ result
     - - HEADER:
           type: RUNINFO
         BODY:
-          min_lsn: 10
+          min_lsn: 11
           max_key: ['ЮЮЮ']
           page_count: 1
           bloom_filter: <bloom_filter>
-          max_lsn: 12
+          max_lsn: 13
           min_key: ['ёёё']
       - HEADER:
           type: PAGEINFO
@@ -288,17 +291,17 @@ result
           min_key: ['ёёё']
   - - 00000000000000000012.run
     - - HEADER:
-          lsn: 10
+          lsn: 11
           type: REPLACE
         BODY:
           tuple: ['ёёё', 123]
       - HEADER:
-          lsn: 12
+          lsn: 13
           type: INSERT
         BODY:
           tuple: ['ююю', 789]
       - HEADER:
-          lsn: 11
+          lsn: 12
           type: INSERT
         BODY:
           tuple: ['ЮЮЮ', 456]
@@ -310,11 +313,11 @@ result
     - - HEADER:
           type: RUNINFO
         BODY:
-          min_lsn: 7
+          min_lsn: 8
           max_key: [null, 'ЭЭЭ']
           page_count: 1
           bloom_filter: <bloom_filter>
-          max_lsn: 9
+          max_lsn: 10
           min_key: [null, 'ёёё']
       - HEADER:
           type: PAGEINFO
@@ -327,17 +330,17 @@ result
           min_key: [null, 'ёёё']
   - - 00000000000000000006.run
     - - HEADER:
-          lsn: 9
+          lsn: 10
           type: INSERT
         BODY:
           tuple: [null, 'ёёё']
       - HEADER:
-          lsn: 8
+          lsn: 9
           type: INSERT
         BODY:
           tuple: [null, 'эээ']
       - HEADER:
-          lsn: 7
+          lsn: 8
           type: INSERT
         BODY:
           tuple: [null, 'ЭЭЭ']
@@ -349,11 +352,11 @@ result
     - - HEADER:
           type: RUNINFO
         BODY:
-          min_lsn: 10
+          min_lsn: 11
           max_key: [789, 'ююю']
           page_count: 1
           bloom_filter: <bloom_filter>
-          max_lsn: 12
+          max_lsn: 13
           min_key: [null, 'ёёё']
       - HEADER:
           type: PAGEINFO
@@ -366,22 +369,22 @@ result
           min_key: [null, 'ёёё']
   - - 00000000000000000010.run
     - - HEADER:
-          lsn: 10
+          lsn: 11
           type: DELETE
         BODY:
           key: [null, 'ёёё']
       - HEADER:
-          lsn: 10
+          lsn: 11
           type: REPLACE
         BODY:
           tuple: [123, 'ёёё']
       - HEADER:
-          lsn: 11
+          lsn: 12
           type: INSERT
         BODY:
           tuple: [456, 'ЮЮЮ']
       - HEADER:
-          lsn: 12
+          lsn: 13
           type: INSERT
         BODY:
           tuple: [789, 'ююю']
diff --git a/test/vinyl/layout.test.lua b/test/vinyl/layout.test.lua
index fd5787b7..27bf5d40 100644
--- a/test/vinyl/layout.test.lua
+++ b/test/vinyl/layout.test.lua
@@ -8,13 +8,15 @@ fun = require 'fun'
 
 space = box.schema.space.create('test', {engine='vinyl'})
 _ = space:create_index('pk', {parts = {{1, 'string', collation = 'unicode'}}, run_count_per_level=3})
-_ = space:create_index('sk', {parts = {{2, 'unsigned', is_nullable = true}}, run_count_per_level=3})
+_ = space:create_index('sk', {parts = {{2, 'unsigned'}}, run_count_per_level=3})
 
 -- Empty run
 space:insert{'ЁЁЁ', 777}
 space:delete{'ЁЁЁ'}
 box.snapshot()
 
+space.index.sk:alter{parts = {{2, 'unsigned', is_nullable = true}}}
+
 space:replace{'ЭЭЭ', box.NULL}
 space:replace{'эээ', box.NULL}
 space:replace{'ёёё', box.NULL}