From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH 06/12] space: space_vtab::build_secondary_key => build_index Date: Sat, 7 Apr 2018 16:38:03 +0300 Message-Id: In-Reply-To: References: In-Reply-To: References: To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: 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