[PATCH 06/12] space: space_vtab::build_secondary_key => build_index

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

The build_secondary_key method of space vtab is used not only for
building secondary indexes, but also for rebuilding primary indexes.
To avoid confusion, let's rename it to build_index and pass to it
the source space, the new index, and the new tuple format.
---
 src/box/alter.cc         | 10 +++++-----
 src/box/memtx_space.c    | 26 ++++++++++++--------------
 src/box/space.h          | 27 +++++++++++++--------------
 src/box/sysview_engine.c | 13 ++++++-------
 src/box/vinyl.c          | 18 ++++++------------
 src/errinj.h             |  2 +-
 test/box/errinj.result   | 34 +++++++++++++++++-----------------
 test/box/errinj.test.lua | 10 +++++-----
 8 files changed, 65 insertions(+), 75 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 9d0c4c23..947716c8 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -1197,8 +1197,8 @@ CreateIndex::alter(struct alter_space *alter)
 	struct index *new_index = space_index(alter->new_space,
 					      new_index_def->iid);
 	assert(new_index != NULL);
-	space_build_secondary_key_xc(alter->new_space,
-				     alter->new_space, new_index);
+	space_build_index_xc(alter->new_space, new_index,
+			     alter->new_space->format);
 }
 
 void
@@ -1270,9 +1270,9 @@ RebuildIndex::alter(struct alter_space *alter)
 	struct index *new_index = space_index(alter->new_space,
 					      new_index_def->iid);
 	assert(new_index != NULL);
-	space_build_secondary_key_xc(new_index_def->iid != 0 ?
-				     alter->new_space : alter->old_space,
-				     alter->new_space, new_index);
+	space_build_index_xc(new_index_def->iid != 0 ?
+			     alter->new_space : alter->old_space,
+			     new_index, alter->new_space->format);
 }
 
 void
diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index b3e08e49..ca7b5937 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -754,27 +754,26 @@ memtx_init_system_space(struct space *space)
 }
 
 static int
-memtx_space_build_secondary_key(struct space *old_space,
-				struct space *new_space,
-				struct index *new_index)
+memtx_space_build_index(struct space *space, struct index *index,
+			struct tuple_format *format)
 {
 	/**
 	 * If it's a secondary key, and we're not building them
 	 * yet (i.e. it's snapshot recovery for memtx), do nothing.
 	 */
-	if (new_index->def->iid != 0) {
+	if (index->def->iid != 0) {
 		struct memtx_space *memtx_space;
-		memtx_space = (struct memtx_space *)new_space;
+		memtx_space = (struct memtx_space *)space;
 		if (!(memtx_space->replace == memtx_space_replace_all_keys))
 			return 0;
 	}
-	struct index *pk = index_find(old_space, 0);
+	struct index *pk = index_find(space, 0);
 	if (pk == NULL)
 		return -1;
 
-	struct errinj *inj = errinj(ERRINJ_BUILD_SECONDARY, ERRINJ_INT);
-	if (inj != NULL && inj->iparam == (int)new_index->def->iid) {
-		diag_set(ClientError, ER_INJECTION, "buildSecondaryKey");
+	struct errinj *inj = errinj(ERRINJ_BUILD_INDEX, ERRINJ_INT);
+	if (inj != NULL && inj->iparam == (int)index->def->iid) {
+		diag_set(ClientError, ER_INJECTION, "build index");
 		return -1;
 	}
 
@@ -798,15 +797,14 @@ memtx_space_build_secondary_key(struct space *old_space,
 		 * Check that the tuple is OK according to the
 		 * new format.
 		 */
-		rc = tuple_validate(new_space->format, tuple);
+		rc = tuple_validate(format, tuple);
 		if (rc != 0)
 			break;
 		/*
 		 * @todo: better message if there is a duplicate.
 		 */
 		struct tuple *old_tuple;
-		rc = index_replace(new_index, NULL, tuple,
-				   DUP_INSERT, &old_tuple);
+		rc = index_replace(index, NULL, tuple, DUP_INSERT, &old_tuple);
 		if (rc != 0)
 			break;
 		assert(old_tuple == NULL); /* Guaranteed by DUP_INSERT. */
@@ -815,7 +813,7 @@ memtx_space_build_secondary_key(struct space *old_space,
 		 * All tuples stored in a memtx space must be
 		 * referenced by the primary index.
 		 */
-		if (new_index->def->iid == 0)
+		if (index->def->iid == 0)
 			tuple_ref(tuple);
 	}
 	iterator_delete(it);
@@ -851,7 +849,7 @@ static const struct space_vtab memtx_space_vtab = {
 	/* .add_primary_key = */ memtx_space_add_primary_key,
 	/* .drop_primary_key = */ memtx_space_drop_primary_key,
 	/* .check_format  = */ memtx_space_check_format,
-	/* .build_secondary_key = */ memtx_space_build_secondary_key,
+	/* .build_index = */ memtx_space_build_index,
 	/* .swap_index = */ generic_space_swap_index,
 	/* .prepare_alter = */ memtx_space_prepare_alter,
 };
diff --git a/src/box/space.h b/src/box/space.h
index 758d575e..0cc1b00f 100644
--- a/src/box/space.h
+++ b/src/box/space.h
@@ -48,6 +48,7 @@ struct txn;
 struct request;
 struct port;
 struct tuple;
+struct tuple_format;
 
 struct space_vtab {
 	/** Free a space instance. */
@@ -96,13 +97,13 @@ struct space_vtab {
 	int (*check_format)(struct space *new_space,
 			    struct space *old_space);
 	/**
-	 * Called with the new empty secondary index.
-	 * Fill the new index with data from the primary
-	 * key of the space.
+	 * Build a new index, primary or secondary, and fill it
+	 * with tuples stored in the given space. The function is
+	 * supposed to assure that all tuples conform to the new
+	 * format.
 	 */
-	int (*build_secondary_key)(struct space *old_space,
-				   struct space *new_space,
-				   struct index *new_index);
+	int (*build_index)(struct space *space, struct index *index,
+			   struct tuple_format *format);
 	/**
 	 * Exchange two index objects in two spaces. Used
 	 * to update a space with a newly built index, while
@@ -329,12 +330,10 @@ space_drop_primary_key(struct space *space)
 }
 
 static inline int
-space_build_secondary_key(struct space *old_space,
-			  struct space *new_space, struct index *new_index)
+space_build_index(struct space *space, struct index *index,
+		  struct tuple_format *format)
 {
-	assert(old_space->vtab == new_space->vtab);
-	return new_space->vtab->build_secondary_key(old_space,
-						    new_space, new_index);
+	return space->vtab->build_index(space, index, format);
 }
 
 static inline void
@@ -480,10 +479,10 @@ space_check_format_xc(struct space *new_space, struct space *old_space)
 }
 
 static inline void
-space_build_secondary_key_xc(struct space *old_space,
-			     struct space *new_space, struct index *new_index)
+space_build_index_xc(struct space *space, struct index *index,
+		     struct tuple_format *format)
 {
-	if (space_build_secondary_key(old_space, new_space, new_index) != 0)
+	if (space_build_index(space, index, format) != 0)
 		diag_raise();
 }
 
diff --git a/src/box/sysview_engine.c b/src/box/sysview_engine.c
index d28430aa..8cfdeeba 100644
--- a/src/box/sysview_engine.c
+++ b/src/box/sysview_engine.c
@@ -136,13 +136,12 @@ sysview_space_drop_primary_key(struct space *space)
 }
 
 static int
-sysview_space_build_secondary_key(struct space *old_space,
-				  struct space *new_space,
-				  struct index *new_index)
+sysview_space_build_index(struct space *space, struct index *index,
+			  struct tuple_format *format)
 {
-	(void)old_space;
-	(void)new_space;
-	(void)new_index;
+	(void)space;
+	(void)index;
+	(void)format;
 	return 0;
 }
 
@@ -177,7 +176,7 @@ static const struct space_vtab sysview_space_vtab = {
 	/* .add_primary_key = */ sysview_space_add_primary_key,
 	/* .drop_primary_key = */ sysview_space_drop_primary_key,
 	/* .check_format = */ sysview_space_check_format,
-	/* .build_secondary_key = */ sysview_space_build_secondary_key,
+	/* .build_index = */ sysview_space_build_index,
 	/* .swap_index = */ generic_space_swap_index,
 	/* .prepare_alter = */ sysview_space_prepare_alter,
 };
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index e809f6a8..7db7a60f 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -1094,11 +1094,10 @@ vinyl_space_drop_primary_key(struct space *space)
 }
 
 static int
-vinyl_space_build_secondary_key(struct space *old_space,
-				struct space *new_space,
-				struct index *new_index)
+vinyl_space_build_index(struct space *space, struct index *index,
+			struct tuple_format *format)
 {
-	(void)old_space;
+	(void)format;
 	/*
 	 * Unlike Memtx, Vinyl does not need building of a secondary index.
 	 * This is true because of two things:
@@ -1113,17 +1112,12 @@ vinyl_space_build_secondary_key(struct space *old_space,
 	 * by itself during WAL recovery.
 	 * III. Vinyl is online. The space is definitely empty and there's
 	 * nothing to build.
-	 *
-	 * When we start to implement alter of non-empty vinyl spaces, it
-	 *  seems that we should call here:
-	 *   Engine::buildSecondaryKey(old_space, new_space, new_index_arg);
-	 *  but aware of three cases mentioned above.
 	 */
-	if (vinyl_index_open(new_index) != 0)
+	if (vinyl_index_open(index) != 0)
 		return -1;
 
 	/* Set pointer to the primary key for the new index. */
-	vy_lsm_update_pk(vy_lsm(new_index), vy_lsm(space_index(new_space, 0)));
+	vy_lsm_update_pk(vy_lsm(index), vy_lsm(space_index(space, 0)));
 	return 0;
 }
 
@@ -3931,7 +3925,7 @@ static const struct space_vtab vinyl_space_vtab = {
 	/* .add_primary_key = */ vinyl_space_add_primary_key,
 	/* .drop_primary_key = */ vinyl_space_drop_primary_key,
 	/* .check_format = */ vinyl_space_check_format,
-	/* .build_secondary_key = */ vinyl_space_build_secondary_key,
+	/* .build_index = */ vinyl_space_build_index,
 	/* .swap_index = */ vinyl_space_swap_index,
 	/* .prepare_alter = */ vinyl_space_prepare_alter,
 };
diff --git a/src/errinj.h b/src/errinj.h
index ab5988cd..2f9d2408 100644
--- a/src/errinj.h
+++ b/src/errinj.h
@@ -102,7 +102,7 @@ struct errinj {
 	_(ERRINJ_XLOG_READ, ERRINJ_INT, {.iparam = -1}) \
 	_(ERRINJ_VYRUN_INDEX_GARBAGE, ERRINJ_BOOL, {.bparam = false}) \
 	_(ERRINJ_VYRUN_DATA_READ, ERRINJ_BOOL, {.bparam = false}) \
-	_(ERRINJ_BUILD_SECONDARY, ERRINJ_INT, {.iparam = -1}) \
+	_(ERRINJ_BUILD_INDEX, ERRINJ_INT, {.iparam = -1}) \
 	_(ERRINJ_VY_POINT_ITER_WAIT, ERRINJ_BOOL, {.bparam = false}) \
 	_(ERRINJ_RELAY_EXIT_DELAY, ERRINJ_DOUBLE, {.dparam = 0}) \
 	_(ERRINJ_VY_DELAY_PK_LOOKUP, ERRINJ_BOOL, {.bparam = false}) \
diff --git a/test/box/errinj.result b/test/box/errinj.result
index 92ae7235..958f82dd 100644
--- a/test/box/errinj.result
+++ b/test/box/errinj.result
@@ -56,6 +56,8 @@ errinj.info()
     state: false
   ERRINJ_VY_RUN_WRITE:
     state: false
+  ERRINJ_BUILD_INDEX:
+    state: -1
   ERRINJ_RELAY_FINAL_SLEEP:
     state: false
   ERRINJ_VY_RUN_DISCARD:
@@ -64,30 +66,28 @@ errinj.info()
     state: false
   ERRINJ_LOG_ROTATE:
     state: false
-  ERRINJ_VY_POINT_ITER_WAIT:
-    state: false
   ERRINJ_RELAY_EXIT_DELAY:
     state: 0
   ERRINJ_IPROTO_TX_DELAY:
     state: false
-  ERRINJ_TUPLE_FIELD:
+  ERRINJ_VY_POINT_ITER_WAIT:
     state: false
-  ERRINJ_INDEX_ALLOC:
+  ERRINJ_TUPLE_FIELD:
     state: false
   ERRINJ_XLOG_GARBAGE:
     state: false
-  ERRINJ_VY_RUN_WRITE_TIMEOUT:
-    state: 0
+  ERRINJ_INDEX_ALLOC:
+    state: false
   ERRINJ_RELAY_TIMEOUT:
     state: 0
   ERRINJ_TESTING:
     state: false
-  ERRINJ_VY_LOG_FLUSH:
-    state: false
+  ERRINJ_VY_RUN_WRITE_TIMEOUT:
+    state: 0
   ERRINJ_VY_SQUASH_TIMEOUT:
     state: 0
-  ERRINJ_BUILD_SECONDARY:
-    state: -1
+  ERRINJ_VY_LOG_FLUSH:
+    state: false
   ERRINJ_VY_INDEX_DUMP:
     state: -1
 ...
@@ -949,13 +949,13 @@ i3:select{}
   - [2, 3, 1, 2, 10, 10]
   - [1, 4, 3, 4, 10, 10]
 ...
-box.error.injection.set('ERRINJ_BUILD_SECONDARY', i2.id)
+box.error.injection.set('ERRINJ_BUILD_INDEX', i2.id)
 ---
 - ok
 ...
 i1:alter{parts = {3, "unsigned"}}
 ---
-- error: Error injection 'buildSecondaryKey'
+- error: Error injection 'build index'
 ...
 _ = collectgarbage('collect')
 ---
@@ -981,13 +981,13 @@ i3:select{}
   - [2, 3, 1, 2, 10, 10]
   - [1, 4, 3, 4, 10, 10]
 ...
-box.error.injection.set('ERRINJ_BUILD_SECONDARY', i3.id)
+box.error.injection.set('ERRINJ_BUILD_INDEX', i3.id)
 ---
 - ok
 ...
 i1:alter{parts = {4, "unsigned"}}
 ---
-- error: Error injection 'buildSecondaryKey'
+- error: Error injection 'build index'
 ...
 _ = collectgarbage('collect')
 ---
@@ -1013,7 +1013,7 @@ i3:select{}
   - [2, 3, 1, 2, 10, 10]
   - [1, 4, 3, 4, 10, 10]
 ...
-box.error.injection.set('ERRINJ_BUILD_SECONDARY', -1)
+box.error.injection.set('ERRINJ_BUILD_INDEX', -1)
 ---
 - ok
 ...
@@ -1037,14 +1037,14 @@ s:replace{1, 1}
 ---
 - [1, 1]
 ...
-box.error.injection.set('ERRINJ_BUILD_SECONDARY', sk.id)
+box.error.injection.set('ERRINJ_BUILD_INDEX', sk.id)
 ---
 - ok
 ...
 sk:alter({parts = {2, 'number'}})
 ---
 ...
-box.error.injection.set('ERRINJ_BUILD_SECONDARY', -1)
+box.error.injection.set('ERRINJ_BUILD_INDEX', -1)
 ---
 - ok
 ...
diff --git a/test/box/errinj.test.lua b/test/box/errinj.test.lua
index 7dea2769..a3dde845 100644
--- a/test/box/errinj.test.lua
+++ b/test/box/errinj.test.lua
@@ -311,7 +311,7 @@ i1:select{}
 i2:select{}
 i3:select{}
 
-box.error.injection.set('ERRINJ_BUILD_SECONDARY', i2.id)
+box.error.injection.set('ERRINJ_BUILD_INDEX', i2.id)
 
 i1:alter{parts = {3, "unsigned"}}
 
@@ -320,7 +320,7 @@ i1:select{}
 i2:select{}
 i3:select{}
 
-box.error.injection.set('ERRINJ_BUILD_SECONDARY', i3.id)
+box.error.injection.set('ERRINJ_BUILD_INDEX', i3.id)
 
 i1:alter{parts = {4, "unsigned"}}
 
@@ -329,7 +329,7 @@ i1:select{}
 i2:select{}
 i3:select{}
 
-box.error.injection.set('ERRINJ_BUILD_SECONDARY', -1)
+box.error.injection.set('ERRINJ_BUILD_INDEX', -1)
 
 s:drop()
 
@@ -341,9 +341,9 @@ s = box.schema.space.create('test')
 pk = s:create_index('pk')
 sk = s:create_index('sk', {parts = {2, 'unsigned'}})
 s:replace{1, 1}
-box.error.injection.set('ERRINJ_BUILD_SECONDARY', sk.id)
+box.error.injection.set('ERRINJ_BUILD_INDEX', sk.id)
 sk:alter({parts = {2, 'number'}})
-box.error.injection.set('ERRINJ_BUILD_SECONDARY', -1)
+box.error.injection.set('ERRINJ_BUILD_INDEX', -1)
 s:drop()
 
 --
-- 
2.11.0