From: Vladimir Davydov <vdavydov.dev@gmail.com> To: tarantool-patches@freelists.org Subject: [PATCH 1/3] vinyl: use update_def index method to update vy_lsm on ddl Date: Tue, 23 Jul 2019 18:49:26 +0300 [thread overview] Message-ID: <20a1aafbfba6068f17a07b01e4c829db0d429fe0.1563895921.git.vdavydov.dev@gmail.com> (raw) In-Reply-To: <cover.1563895921.git.vdavydov.dev@gmail.com> When an index definition is modified by DDL in such a way that doesn't require index rebuild (e.g. a key part type is changed from unsigned to integer), we move the index from the old space container to the new one, see ModifyIndex. In case of Vinyl we also need to update key definitions and options stored in vy_lsm. We do that in swap_index space method, but this isn't entirely correct, as this method is also called when an index is moved without any modifications, see MoveIndex. Let's do this from update_def index method instead, which seems to be more suitable. The only reason this code lives in swap_index space method now is that we didn't have update_def index method when this functionality was introduced in the first place. --- src/box/key_def.c | 56 ++++++++++++++++++++++++++++------------------- src/box/key_def.h | 4 ++-- src/box/vinyl.c | 14 ++++++++---- 3 files changed, 46 insertions(+), 28 deletions(-) diff --git a/src/box/key_def.c b/src/box/key_def.c index ee758eef..9fa1ca49 100644 --- a/src/box/key_def.c +++ b/src/box/key_def.c @@ -81,18 +81,26 @@ const struct opt_def part_def_reg[] = { OPT_END, }; -struct key_def * -key_def_dup(const struct key_def *src) +/** + * Return the size of memory occupied by the given key definition. + */ +static inline size_t +key_def_copy_size(const struct key_def *def) { size_t sz = 0; - for (uint32_t i = 0; i < src->part_count; i++) - sz += src->parts[i].path_len; - sz = key_def_sizeof(src->part_count, sz); - struct key_def *res = (struct key_def *)malloc(sz); - if (res == NULL) { - diag_set(OutOfMemory, sz, "malloc", "res"); - return NULL; - } + for (uint32_t i = 0; i < def->part_count; i++) + sz += def->parts[i].path_len; + return key_def_sizeof(def->part_count, sz); +} + +/** + * A helper function for key_def_copy() and key_def_dup() that + * copies key definition src of size sz to res without checking + * that the two key definitions have the same allocation size. + */ +static struct key_def * +key_def_do_copy(struct key_def *res, const struct key_def *src, size_t sz) +{ memcpy(res, src, sz); /* * Update the paths pointers so that they refer to the @@ -112,20 +120,24 @@ key_def_dup(const struct key_def *src) } void -key_def_swap(struct key_def *old_def, struct key_def *new_def) +key_def_copy(struct key_def *dest, const struct key_def *src) { - 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]); - /* - * Paths are allocated as a part of key_def so - * we need to swap path pointers back - it's OK - * as paths aren't supposed to change. - */ - assert(old_def->parts[i].path_len == new_def->parts[i].path_len); - SWAP(old_def->parts[i].path, new_def->parts[i].path); + size_t sz = key_def_copy_size(src); + assert(sz = key_def_copy_size(dest)); + key_def_do_copy(dest, src, sz); +} + +struct key_def * +key_def_dup(const struct key_def *src) +{ + size_t sz = key_def_copy_size(src); + struct key_def *res = malloc(sz); + if (res == NULL) { + diag_set(OutOfMemory, sz, "malloc", "res"); + return NULL; } - SWAP(*old_def, *new_def); + key_def_do_copy(res, src, sz); + return res; } void diff --git a/src/box/key_def.h b/src/box/key_def.h index df83d055..73aefb9a 100644 --- a/src/box/key_def.h +++ b/src/box/key_def.h @@ -250,11 +250,11 @@ struct key_def * key_def_dup(const struct key_def *src); /** - * Swap content of two key definitions in memory. + * Copy content of key definition src to dest. * The two key definitions must have the same size. */ void -key_def_swap(struct key_def *old_def, struct key_def *new_def); +key_def_copy(struct key_def *dest, const struct key_def *src); /** * Delete @a key_def. diff --git a/src/box/vinyl.c b/src/box/vinyl.c index cf7af26b..4ac54138 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -963,6 +963,15 @@ vinyl_index_commit_drop(struct index *index, int64_t lsn) vy_log_tx_try_commit(); } +static void +vinyl_index_update_def(struct index *index) +{ + struct vy_lsm *lsm = vy_lsm(index); + lsm->opts = index->def->opts; + key_def_copy(lsm->key_def, index->def->key_def); + key_def_copy(lsm->cmp_def, index->def->cmp_def); +} + static bool vinyl_index_depends_on_pk(struct index *index) { @@ -1200,9 +1209,6 @@ vinyl_space_swap_index(struct space *old_space, struct space *new_space, SWAP(old_lsm->check_is_unique, new_lsm->check_is_unique); SWAP(old_lsm->mem_format, new_lsm->mem_format); 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); /* Update pointer to the primary key. */ vy_lsm_update_pk(old_lsm, vy_lsm(old_space->index_map[0])); @@ -4741,7 +4747,7 @@ static const struct index_vtab vinyl_index_vtab = { /* .abort_create = */ vinyl_index_abort_create, /* .commit_modify = */ vinyl_index_commit_modify, /* .commit_drop = */ vinyl_index_commit_drop, - /* .update_def = */ generic_index_update_def, + /* .update_def = */ vinyl_index_update_def, /* .depends_on_pk = */ vinyl_index_depends_on_pk, /* .def_change_requires_rebuild = */ vinyl_index_def_change_requires_rebuild, -- 2.20.1
next prev parent reply other threads:[~2019-07-23 15:49 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-23 15:49 [PATCH 0/3] Fix crash on rollback of mixed DML/DDL transaction Vladimir Davydov 2019-07-23 15:49 ` Vladimir Davydov [this message] 2019-07-24 22:25 ` [tarantool-patches] Re: [PATCH 1/3] vinyl: use update_def index method to update vy_lsm on ddl Konstantin Osipov 2019-07-25 8:38 ` Vladimir Davydov 2019-07-25 12:11 ` Vladimir Davydov 2019-07-23 15:49 ` [PATCH 2/3] vinyl: move unique check optimization setup to the space level Vladimir Davydov 2019-07-24 22:31 ` [tarantool-patches] " Konstantin Osipov 2019-07-25 8:46 ` Vladimir Davydov 2019-07-25 12:11 ` Vladimir Davydov 2019-07-23 15:49 ` [PATCH 3/3] alter: fix rollback in case DDL and DML are used in the same transaction Vladimir Davydov 2019-07-24 22:32 ` [tarantool-patches] " Konstantin Osipov 2019-07-25 8:51 ` Vladimir Davydov 2019-07-30 10:56 ` Vladimir Davydov 2019-07-30 12:14 ` Konstantin Osipov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20a1aafbfba6068f17a07b01e4c829db0d429fe0.1563895921.git.vdavydov.dev@gmail.com \ --to=vdavydov.dev@gmail.com \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH 1/3] vinyl: use update_def index method to update vy_lsm on ddl' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox