From c0c5be2bae512a1c48027fa1519edd6cb168bd59 Mon Sep 17 00:00:00 2001
From: Alexandr Lyapunov <aleks@tarantool.org>
Date: Fri, 7 Jul 2017 17:19:55 +0300
Subject: [PATCH] alter: rebuild some secondary indexes if the primary was
changed
Since the secondary indexes order will be able to depend on
the order in the primary index, we need to rebuild them if the
primary index was changed.
Only non-unique tree indexes depend on the primary index, so rebuild
only non-unique tree indexes.
Add a test.
Needed for #2476
---
src/box/alter.cc | 38 ++++++++++--
src/box/memtx_space.cc | 5 ++
src/errinj.h | 1 +
test/box/errinj.result | 124 +++++++++++++++++++++++++++++++++++++++
test/box/errinj.test.lua | 48 +++++++++++++++
5 files changed, 212 insertions(+), 4 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index d6a41df9a1..e342d1132a 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -535,6 +535,12 @@ struct alter_space {
struct space *old_space;
/** New space. */
struct space *new_space;
+ /**
+ * Assigned to the new primary key definition if we're
+ * rebuilding the primary key, i.e. changing its key parts
+ * substantially.
+ */
+ struct key_def *pk_def;
};
struct alter_space *
@@ -1017,7 +1023,12 @@ class RebuildIndex: public AlterSpaceOp {
struct index_def *old_index_def_arg)
:AlterSpaceOp(alter),
new_index_def(new_index_def_arg),
- old_index_def(old_index_def_arg) {}
+ old_index_def(old_index_def_arg)
+ {
+ /* We may want to rebuild secondary keys as well. */
+ if (new_index_def->iid == 0)
+ alter->pk_def = new_index_def->key_def;
+ }
/** New index index_def. */
struct index_def *new_index_def;
/** Old index index_def. */
@@ -1108,10 +1119,29 @@ alter_space_move_indexes(struct alter_space *alter, uint32_t begin,
{
struct space *old_space = alter->old_space;
for (uint32_t index_id = begin; index_id < end; ++index_id) {
- Index *index = space_index(old_space, index_id);
- if (index == NULL)
+ Index *old_index = space_index(old_space, index_id);
+ if (old_index == NULL)
continue;
- (void) new MoveIndex(alter, index->index_def->iid);
+ struct index_def *old_def = old_index->index_def;
+ if (old_def->opts.is_unique || old_def->type != TREE ||
+ alter->pk_def == NULL) {
+
+ (void) new MoveIndex(alter, old_def->iid);
+ continue;
+ }
+ /*
+ * Rebuild non-unique secondary keys along with
+ * the primary, since primary key parts have
+ * changed.
+ */
+ struct index_def *new_def =
+ index_def_new(old_def->space_id, old_def->iid,
+ old_def->name, strlen(old_def->name),
+ old_def->type, &old_def->opts,
+ old_def->key_def, alter->pk_def);
+ auto guard = make_scoped_guard([=] { index_def_delete(new_def); });
+ (void) new RebuildIndex(alter, new_def, old_def);
+ guard.is_active = false;
}
}
diff --git a/src/box/memtx_space.cc b/src/box/memtx_space.cc
index 077ec60850..fda15e0429 100644
--- a/src/box/memtx_space.cc
+++ b/src/box/memtx_space.cc
@@ -680,6 +680,11 @@ MemtxSpace::buildSecondaryKey(struct space *old_space,
}
Index *pk = index_find_xc(old_space, 0);
+ struct errinj *inj = errinj(ERRINJ_BUILD_SECONDARY, ERRINJ_INT);
+ if (inj != NULL && inj->iparam == (int)new_index->index_def->iid) {
+ tnt_raise(ClientError, ER_INJECTION, "buildSecondaryKey");
+ }
+
/* Now deal with any kind of add index during normal operation. */
struct iterator *it = pk->allocIterator();
IteratorGuard guard(it);
diff --git a/src/errinj.h b/src/errinj.h
index 298f444eb4..85a6976546 100644
--- a/src/errinj.h
+++ b/src/errinj.h
@@ -102,6 +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}) \
ENUM0(errinj_id, ERRINJ_LIST);
extern struct errinj errinjs[];
diff --git a/test/box/errinj.result b/test/box/errinj.result
index 7212bd0b7e..97ce7ee24f 100644
--- a/test/box/errinj.result
+++ b/test/box/errinj.result
@@ -16,6 +16,8 @@ errinj.info()
state: false
ERRINJ_WAL_WRITE:
state: false
+ ERRINJ_BUILD_SECONDARY:
+ state: -1
ERRINJ_VY_READ_PAGE_TIMEOUT:
state: false
ERRINJ_XLOG_META:
@@ -862,3 +864,125 @@ errinj.set("ERRINJ_WAL_WRITE", false)
s:drop()
---
...
+-- rebuild some secondary indexes if the primary was changed
+s = box.schema.space.create('test')
+---
+...
+i1 = s:create_index('i1', {parts = {1, 'unsigned'}})
+---
+...
+--i2 = s:create_index('i2', {parts = {5, 'unsigned'}, unique = false})
+--i3 = s:create_index('i3', {parts = {6, 'unsigned'}, unique = false})
+i2 = i1 i3 = i1
+---
+...
+_ = s:insert{1, 4, 3, 4, 10, 10}
+---
+...
+_ = s:insert{2, 3, 1, 2, 10, 10}
+---
+...
+_ = s:insert{3, 2, 2, 1, 10, 10}
+---
+...
+_ = s:insert{4, 1, 4, 3, 10, 10}
+---
+...
+function sum(select_res) local r = 0 for _,t in pairs(select_res) do r = r + t[1] end return r end
+---
+...
+i1:select{}
+---
+- - [1, 4, 3, 4, 10, 10]
+ - [2, 3, 1, 2, 10, 10]
+ - [3, 2, 2, 1, 10, 10]
+ - [4, 1, 4, 3, 10, 10]
+...
+sum(i2:select{})
+---
+- 10
+...
+sum(i3:select{})
+---
+- 10
+...
+i1:alter({parts={2, 'unsigned'}})
+---
+...
+_ = collectgarbage('collect')
+---
+...
+i1:select{}
+---
+- - [4, 1, 4, 3, 10, 10]
+ - [3, 2, 2, 1, 10, 10]
+ - [2, 3, 1, 2, 10, 10]
+ - [1, 4, 3, 4, 10, 10]
+...
+sum(i2:select{})
+---
+- 10
+...
+sum(i3:select{})
+---
+- 10
+...
+box.error.injection.set('ERRINJ_BUILD_SECONDARY', i2.id)
+---
+- ok
+...
+i1:alter{parts = {3, "unsigned"}}
+---
+- error: Error injection 'buildSecondaryKey'
+...
+_ = collectgarbage('collect')
+---
+...
+i1:select{}
+---
+- - [4, 1, 4, 3, 10, 10]
+ - [3, 2, 2, 1, 10, 10]
+ - [2, 3, 1, 2, 10, 10]
+ - [1, 4, 3, 4, 10, 10]
+...
+sum(i2:select{})
+---
+- 10
+...
+sum(i3:select{})
+---
+- 10
+...
+box.error.injection.set('ERRINJ_BUILD_SECONDARY', i3.id)
+---
+- ok
+...
+i1:alter{parts = {4, "unsigned"}}
+---
+- error: Error injection 'buildSecondaryKey'
+...
+_ = collectgarbage('collect')
+---
+...
+i1:select{}
+---
+- - [4, 1, 4, 3, 10, 10]
+ - [3, 2, 2, 1, 10, 10]
+ - [2, 3, 1, 2, 10, 10]
+ - [1, 4, 3, 4, 10, 10]
+...
+sum(i2:select{})
+---
+- 10
+...
+sum(i3:select{})
+---
+- 10
+...
+box.error.injection.set('ERRINJ_BUILD_SECONDARY', -1)
+---
+- ok
+...
+s:drop()
+---
+...
diff --git a/test/box/errinj.test.lua b/test/box/errinj.test.lua
index cca3f5b04b..97d4ac2d67 100644
--- a/test/box/errinj.test.lua
+++ b/test/box/errinj.test.lua
@@ -285,3 +285,51 @@ _ = {fiber.create(test, {1, 2, 3}), fiber.create(test, {3, 4, 5})}
{ch:get(), ch:get()}
errinj.set("ERRINJ_WAL_WRITE", false)
s:drop()
+
+-- rebuild some secondary indexes if the primary was changed
+s = box.schema.space.create('test')
+i1 = s:create_index('i1', {parts = {1, 'unsigned'}})
+--i2 = s:create_index('i2', {parts = {5, 'unsigned'}, unique = false})
+--i3 = s:create_index('i3', {parts = {6, 'unsigned'}, unique = false})
+i2 = i1 i3 = i1
+
+_ = s:insert{1, 4, 3, 4, 10, 10}
+_ = s:insert{2, 3, 1, 2, 10, 10}
+_ = s:insert{3, 2, 2, 1, 10, 10}
+_ = s:insert{4, 1, 4, 3, 10, 10}
+
+
+function sum(select_res) local r = 0 for _,t in pairs(select_res) do r = r + t[1] end return r end
+
+i1:select{}
+sum(i2:select{})
+sum(i3:select{})
+
+i1:alter({parts={2, 'unsigned'}})
+
+_ = collectgarbage('collect')
+i1:select{}
+sum(i2:select{})
+sum(i3:select{})
+
+box.error.injection.set('ERRINJ_BUILD_SECONDARY', i2.id)
+
+i1:alter{parts = {3, "unsigned"}}
+
+_ = collectgarbage('collect')
+i1:select{}
+sum(i2:select{})
+sum(i3:select{})
+
+box.error.injection.set('ERRINJ_BUILD_SECONDARY', i3.id)
+
+i1:alter{parts = {4, "unsigned"}}
+
+_ = collectgarbage('collect')
+i1:select{}
+sum(i2:select{})
+sum(i3:select{})
+
+box.error.injection.set('ERRINJ_BUILD_SECONDARY', -1)
+
+s:drop()
--
GitLab