From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH 02/12] alter: require rebuild of all secondary vinyl indexes if pk changes Date: Sun, 1 Apr 2018 12:05:29 +0300 Message-Id: <497ee3578ed75ceb6f34a792cb7f241889016db4.1522572160.git.vdavydov.dev@gmail.com> In-Reply-To: References: In-Reply-To: References: To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: If the primary key is modified, we schedule rebuild of all non-unique (including nullable) secondary TREE indexes. This is valid for memtx, but is not quite right for vinyl. For vinyl we have to rebuild all secondary indexes, because they are all non-clustered (i.e. point to tuples via primary key parts). This doesn't result in any bugs for now, because rebuild of vinyl indexes is not supported, but hopefully this is going to change soon. So let's introduce a new virtual index method, index_vtab::depends_on_pk, which returns true iff the index needs to be updated if the primary key changes, and define this new method for vinyl and memtx TREE indexes. --- src/box/alter.cc | 11 ++++------- src/box/index.cc | 5 +++++ src/box/index.h | 13 +++++++++++++ src/box/memtx_bitset.c | 1 + src/box/memtx_hash.c | 1 + src/box/memtx_rtree.c | 1 + src/box/memtx_tree.c | 9 +++++++++ src/box/sysview_index.c | 1 + src/box/vinyl.c | 12 ++++++++++++ 9 files changed, 47 insertions(+), 7 deletions(-) diff --git a/src/box/alter.cc b/src/box/alter.cc index eb548e0b..7f130297 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -1100,7 +1100,7 @@ public: old_index_def->key_def->part_count) != 0) { /* * Primary parts have been changed - - * update non-unique secondary indexes. + * update secondary indexes. */ alter->pk_def = new_index_def->key_def; } @@ -1411,9 +1411,7 @@ alter_space_move_indexes(struct alter_space *alter, uint32_t begin, struct index_def *old_def = old_index->def; struct index_def *new_def; uint32_t min_field_count = alter->new_min_field_count; - if ((old_def->opts.is_unique && - !old_def->key_def->is_nullable) || - old_def->type != TREE || alter->pk_def == NULL) { + if (alter->pk_def == NULL || !index_depends_on_pk(old_index)) { if (is_min_field_count_changed) { new_def = index_def_dup(old_def); index_def_update_optionality(new_def, @@ -1425,9 +1423,8 @@ alter_space_move_indexes(struct alter_space *alter, uint32_t begin, continue; } /* - * Rebuild non-unique secondary keys along with - * the primary, since primary key parts have - * changed. + * Rebuild secondary indexes that depend on the + * primary key since primary key parts have changed. */ new_def = index_def_new(old_def->space_id, old_def->iid, old_def->name, strlen(old_def->name), diff --git a/src/box/index.cc b/src/box/index.cc index 11a6683d..6cd04833 100644 --- a/src/box/index.cc +++ b/src/box/index.cc @@ -560,6 +560,11 @@ generic_index_update_def(struct index *) { } +bool generic_index_depends_on_pk(struct index *) +{ + return false; +} + ssize_t generic_index_size(struct index *index) { diff --git a/src/box/index.h b/src/box/index.h index 2e43d999..f19fbd16 100644 --- a/src/box/index.h +++ b/src/box/index.h @@ -373,6 +373,12 @@ struct index_vtab { * require index rebuild. */ void (*update_def)(struct index *); + /** + * Return true if the index depends on the primary + * key definition and hence needs to be updated if + * the primary key is modified. + */ + bool (*depends_on_pk)(struct index *); ssize_t (*size)(struct index *); ssize_t (*bsize)(struct index *); @@ -505,6 +511,12 @@ index_update_def(struct index *index) index->vtab->update_def(index); } +static inline bool +index_depends_on_pk(struct index *index) +{ + return index->vtab->depends_on_pk(index); +} + static inline ssize_t index_size(struct index *index) { @@ -616,6 +628,7 @@ void generic_index_abort_create(struct index *); void generic_index_commit_modify(struct index *, int64_t); void generic_index_commit_drop(struct index *); void generic_index_update_def(struct index *); +bool generic_index_depends_on_pk(struct index *); ssize_t generic_index_size(struct index *); int generic_index_min(struct index *, const char *, uint32_t, struct tuple **); int generic_index_max(struct index *, const char *, uint32_t, struct tuple **); diff --git a/src/box/memtx_bitset.c b/src/box/memtx_bitset.c index 0e546217..e66247ee 100644 --- a/src/box/memtx_bitset.c +++ b/src/box/memtx_bitset.c @@ -461,6 +461,7 @@ static const struct index_vtab memtx_bitset_index_vtab = { /* .commit_modify = */ generic_index_commit_modify, /* .commit_drop = */ generic_index_commit_drop, /* .update_def = */ generic_index_update_def, + /* .depends_on_pk = */ generic_index_depends_on_pk, /* .size = */ memtx_bitset_index_size, /* .bsize = */ memtx_bitset_index_bsize, /* .min = */ generic_index_min, diff --git a/src/box/memtx_hash.c b/src/box/memtx_hash.c index 9c72af5d..c4081edc 100644 --- a/src/box/memtx_hash.c +++ b/src/box/memtx_hash.c @@ -385,6 +385,7 @@ static const struct index_vtab memtx_hash_index_vtab = { /* .commit_modify = */ generic_index_commit_modify, /* .commit_drop = */ generic_index_commit_drop, /* .update_def = */ memtx_hash_index_update_def, + /* .depends_on_pk = */ generic_index_depends_on_pk, /* .size = */ memtx_hash_index_size, /* .bsize = */ memtx_hash_index_bsize, /* .min = */ generic_index_min, diff --git a/src/box/memtx_rtree.c b/src/box/memtx_rtree.c index 373d658e..7cd3ac30 100644 --- a/src/box/memtx_rtree.c +++ b/src/box/memtx_rtree.c @@ -299,6 +299,7 @@ static const struct index_vtab memtx_rtree_index_vtab = { /* .commit_modify = */ generic_index_commit_modify, /* .commit_drop = */ generic_index_commit_drop, /* .update_def = */ generic_index_update_def, + /* .depends_on_pk = */ generic_index_depends_on_pk, /* .size = */ memtx_rtree_index_size, /* .bsize = */ memtx_rtree_index_bsize, /* .min = */ generic_index_min, diff --git a/src/box/memtx_tree.c b/src/box/memtx_tree.c index 7178a4bc..a06b590d 100644 --- a/src/box/memtx_tree.c +++ b/src/box/memtx_tree.c @@ -316,6 +316,14 @@ memtx_tree_index_update_def(struct index *base) index->tree.arg = memtx_tree_index_cmp_def(index); } +static bool +memtx_tree_index_depends_on_pk(struct index *base) +{ + struct index_def *def = base->def; + /* See comment to memtx_tree_index_cmp_def(). */ + return !def->opts.is_unique || def->key_def->is_nullable; +} + static ssize_t memtx_tree_index_size(struct index *base) { @@ -586,6 +594,7 @@ static const struct index_vtab memtx_tree_index_vtab = { /* .commit_modify = */ generic_index_commit_modify, /* .commit_drop = */ generic_index_commit_drop, /* .update_def = */ memtx_tree_index_update_def, + /* .depends_on_pk = */ memtx_tree_index_depends_on_pk, /* .size = */ memtx_tree_index_size, /* .bsize = */ memtx_tree_index_bsize, /* .min = */ generic_index_min, diff --git a/src/box/sysview_index.c b/src/box/sysview_index.c index e2b2fb96..72961401 100644 --- a/src/box/sysview_index.c +++ b/src/box/sysview_index.c @@ -165,6 +165,7 @@ static const struct index_vtab sysview_index_vtab = { /* .commit_modify = */ generic_index_commit_modify, /* .commit_drop = */ generic_index_commit_drop, /* .update_def = */ generic_index_update_def, + /* .depends_on_pk = */ generic_index_depends_on_pk, /* .size = */ generic_index_size, /* .bsize = */ sysview_index_bsize, /* .min = */ generic_index_min, diff --git a/src/box/vinyl.c b/src/box/vinyl.c index cebe1615..1afc6664 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -906,6 +906,17 @@ vinyl_index_commit_drop(struct index *index) vy_log_tx_try_commit(); } +static bool +vinyl_index_depends_on_pk(struct index *index) +{ + (void)index; + /* + * All secondary Vinyl indexes are non-clustered and hence + * have to be updated if the primary key is modified. + */ + return true; +} + static void vinyl_init_system_space(struct space *space) { @@ -3938,6 +3949,7 @@ static const struct index_vtab vinyl_index_vtab = { /* .commit_modify = */ generic_index_commit_modify, /* .commit_drop = */ vinyl_index_commit_drop, /* .update_def = */ generic_index_update_def, + /* .depends_on_pk = */ vinyl_index_depends_on_pk, /* .size = */ vinyl_index_size, /* .bsize = */ vinyl_index_bsize, /* .min = */ generic_index_min, -- 2.11.0