From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: kostja@tarantool.org
Cc: tarantool-patches@freelists.org
Subject: [PATCH 02/12] alter: require rebuild of all secondary vinyl indexes if pk changes
Date: Sun, 1 Apr 2018 12:05:29 +0300 [thread overview]
Message-ID: <497ee3578ed75ceb6f34a792cb7f241889016db4.1522572160.git.vdavydov.dev@gmail.com> (raw)
In-Reply-To: <cover.1522572160.git.vdavydov.dev@gmail.com>
In-Reply-To: <cover.1522572160.git.vdavydov.dev@gmail.com>
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
next prev parent reply other threads:[~2018-04-01 9:05 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-01 9:05 [PATCH 00/12] vinyl: allow to extend key def of non-empty index Vladimir Davydov
2018-04-01 9:05 ` [PATCH 01/12] index: add commit_modify virtual method Vladimir Davydov
2018-04-01 9:05 ` Vladimir Davydov [this message]
2018-04-01 9:05 ` [PATCH 03/12] alter: do not rebuild secondary indexes on compatible pk changes Vladimir Davydov
2018-04-05 12:30 ` Konstantin Osipov
2018-04-01 9:05 ` [PATCH 04/12] space: make space_swap_index method virtual Vladimir Davydov
2018-04-01 9:05 ` [PATCH 05/12] vinyl: do not reallocate tuple formats on alter Vladimir Davydov
2018-04-01 11:12 ` [tarantool-patches] " v.shpilevoy
2018-04-01 11:24 ` Vladimir Davydov
2018-04-01 9:05 ` [PATCH 06/12] vinyl: zap vy_lsm_validate_formats Vladimir Davydov
2018-04-01 9:05 ` [PATCH 07/12] vinyl: zap vy_mem_update_formats Vladimir Davydov
2018-04-01 9:05 ` [PATCH 08/12] vinyl: remove pointless is_nullable initialization for disk_format Vladimir Davydov
2018-04-01 9:05 ` [PATCH 09/12] vinyl: use source tuple format when copying field map Vladimir Davydov
2018-04-01 9:05 ` [PATCH 10/12] vinyl: rename vy_log_record::commit_lsn to create_lsn Vladimir Davydov
2018-04-01 9:05 ` [PATCH 11/12] vinyl: do not write VY_LOG_DUMP_LSM record to snapshot Vladimir Davydov
2018-04-01 9:05 ` [PATCH 12/12] vinyl: allow to modify key definition if it does not require rebuild Vladimir Davydov
2018-04-02 8:46 ` Vladimir Davydov
2018-04-05 14:32 ` Konstantin Osipov
2018-04-05 14:45 ` Konstantin Osipov
2018-04-05 14:48 ` 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=497ee3578ed75ceb6f34a792cb7f241889016db4.1522572160.git.vdavydov.dev@gmail.com \
--to=vdavydov.dev@gmail.com \
--cc=kostja@tarantool.org \
--cc=tarantool-patches@freelists.org \
--subject='Re: [PATCH 02/12] alter: require rebuild of all secondary vinyl indexes if pk changes' \
/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