From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH 12/12] vinyl: allow to modify key definition if it does not require rebuild Date: Sun, 1 Apr 2018 12:05:39 +0300 Message-Id: In-Reply-To: References: MIME-Version: 1.0 In-Reply-To: References: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: To allow extending key definition for non-empty vinyl spaces, this patch performs the following steps: - Revert commit c31dd19a69b5 ("vinyl: forbid vinyl index key definition alter") that forbade any key def alter. It isn't needed anymore. - Update key_def and cmp_def in vinyl_space_swap_index(). We simply swap the definitions between the old and new indexes in memory. Since all vinyl objects reference either vy_lsm::key_def or vy_lsm::cmp_def or both, and the change is compatible (does not change the order for existing tuples), this should work just fine. - Update key definition in vylog on ALTER. For this, we introduce a new vylog record type, VY_LOG_MODIFY_LSM, which updates key definition. To be able to replay it on recovery in case we failed to flush it before restart, we also store the LSN of the WAL record that triggered the ALTER. It also adds the following test cases: - Modify key definition of primary and secondary indexes of a non-empty space (engine/ddl). - Modify key definition before snapshot and relay it to a newly joined replica (engine/replica_join). - Make sure key definition is updated in vylog on ALTER (vinyl/layout). --- src/box/key_def.cc | 9 +++ src/box/key_def.h | 7 +++ src/box/vinyl.c | 74 +++++++++++++++++-------- src/box/vy_log.c | 79 ++++++++++++++++++++++++-- src/box/vy_log.h | 22 ++++++++ src/box/vy_lsm.c | 5 +- src/box/vy_lsm.h | 10 ++-- test/engine/ddl.result | 113 +++++++++++++++++++++++++++++++++++++- test/engine/ddl.test.lua | 35 +++++++++++- test/engine/replica_join.result | 26 ++++++++- test/engine/replica_join.test.lua | 2 + test/engine/truncate.result | 2 +- test/engine/truncate.test.lua | 2 +- test/vinyl/ddl.result | 78 +++++++------------------- test/vinyl/ddl.test.lua | 30 +++------- test/vinyl/gh.result | 2 +- test/vinyl/layout.result | 67 +++++++++++----------- test/vinyl/layout.test.lua | 4 +- 18 files changed, 412 insertions(+), 155 deletions(-) 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..886a00f8 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,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; + } + 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 +997,15 @@ 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 +1109,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 +3959,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: '") ... 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: type: INSERT BODY: - tuple: [5, {0: 2, 2: 10, 9: 12}] + tuple: [5, {0: 2, 2: 10, 9: 13}] - HEADER: timestamp: type: INSERT @@ -207,7 +210,7 @@ result timestamp: type: INSERT BODY: - tuple: [10, {0: 2, 9: 12}] + tuple: [10, {0: 2, 9: 13}] - HEADER: timestamp: type: INSERT @@ -217,7 +220,7 @@ result timestamp: type: INSERT BODY: - tuple: [5, {2: 12, 9: 12}] + tuple: [5, {2: 12, 9: 13}] - HEADER: timestamp: type: INSERT @@ -227,16 +230,16 @@ result 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: - 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: - 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: - 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: - 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} -- 2.11.0