[PATCH 06/12] space: space_vtab::build_secondary_key => build_index
Vladimir Davydov
vdavydov.dev at gmail.com
Sat Apr 7 16:38:03 MSK 2018
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
More information about the Tarantool-patches
mailing list