[tarantool-patches] [PATCH] Do not update schema_version on space:truncate().
Serge Petrenko
sergepetrenko at tarantool.org
Fri Jul 6 14:46:25 MSK 2018
Schema version is used by both clients and internal modules to check
whether there vere any updates in spaces and indices. While clients
only need to be notified when there is a noticeable change, e.g.
space is removed, internal components also need to be notified when
something like space:truncate() happens, because even though this
operation doesn't change space id or any of its indices, it creates a
new space object, so all the pointers to the old object have to be updated.
Currently both clients and internals share the same schema version, which
leads to unnecessary updates on the client side.
Fix this by implementing 2 separate counters for internal and public use:
schema_state gets updated on every change, including recreation of the same
space object, while schema_version is updated only when there are noticable
changes for the clients. Introduce a new AlterOp to alter.cc to update
public schema_version.
Now all the internals reference schema_state, while all the clients use
schema_version. box.iternal.schema_version() returns schema_version
(the public one).
Closes: #3414
---
https://github.com/tarantool/tarantool/compare/sergepetrenko/gh-3414-fix-schema-version-increase
https://github.com/tarantool/tarantool/issues/3414
src/box/alter.cc | 36 ++++++++++++++++++++++++++++
src/box/index.cc | 10 ++++----
src/box/index.h | 10 ++++----
src/box/schema.cc | 12 ++++++++--
src/box/schema.h | 7 +++---
src/box/sysview_index.c | 2 +-
src/box/vy_index.c | 4 ++--
src/box/vy_mem.c | 4 ++--
src/box/vy_mem.h | 8 +++----
src/box/vy_tx.c | 6 ++---
test/engine/ddl.result | 57 +++++++++++++++++++++++++++++++++++++++++++++
test/engine/ddl.test.lua | 24 +++++++++++++++++++
test/unit/vy_point_lookup.c | 1 +
13 files changed, 154 insertions(+), 27 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index 5dec2519d..8fd907c17 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -1305,15 +1305,42 @@ RebuildIndex::~RebuildIndex()
index_def_delete(new_index_def);
}
+
+/**
+ * UpdateSchemaVersion - increment schema_version. Used on
+ * commit of alter_space_do(), i.e. when creating or dropping
+ * an index, altering a space.
+ */
+class UpdateSchemaVersion: public AlterSpaceOp
+{
+public:
+ UpdateSchemaVersion(struct alter_space * alter)
+ :AlterSpaceOp(alter) {}
+ virtual void commit(struct alter_space *alter, int64_t signature);
+};
+
+void
+UpdateSchemaVersion::commit(struct alter_space *alter, int64_t signature)
+{
+ (void)alter;
+ (void)signature;
+ ++schema_version;
+}
+
/* }}} */
/**
* Delete the space. It is already removed from the space cache.
+ * Also remember to explicitly increment schema_version, because
+ * no AlterOps are registered on dropping a space.
*/
static void
on_drop_space_commit(struct trigger *trigger, void *event)
{
(void) event;
+
+ ++schema_version;
+
struct space *space = (struct space *)trigger->data;
trigger_run_xc(&on_alter_space, space);
space_delete(space);
@@ -1334,11 +1361,16 @@ on_drop_space_rollback(struct trigger *trigger, void *event)
/**
* Run the triggers registered on commit of a change in _space.
+ * Also remember to explicitly increment schema_version, because
+ * no AlterOps are registered on creating a space.
*/
static void
on_create_space_commit(struct trigger *trigger, void *event)
{
(void) event;
+
+ ++schema_version;
+
struct space *space = (struct space *)trigger->data;
trigger_run_xc(&on_alter_space, space);
}
@@ -1603,6 +1635,8 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
def_guard.is_active = false;
/* Create MoveIndex ops for all space indexes. */
alter_space_move_indexes(alter, 0, old_space->index_id_max + 1);
+ /* Remember to update schema_version. */
+ (void) new UpdateSchemaVersion(alter);
alter_space_do(txn, alter);
alter_guard.is_active = false;
}
@@ -1800,6 +1834,8 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)
* old space.
*/
alter_space_move_indexes(alter, iid + 1, old_space->index_id_max + 1);
+ /* Add an op to update schema_version on commit. */
+ (void) new UpdateSchemaVersion(alter);
alter_space_do(txn, alter);
scoped_guard.is_active = false;
}
diff --git a/src/box/index.cc b/src/box/index.cc
index 69fc76116..30cc0a684 100644
--- a/src/box/index.cc
+++ b/src/box/index.cc
@@ -433,7 +433,7 @@ iterator_create(struct iterator *it, struct index *index)
{
it->next = NULL;
it->free = NULL;
- it->schema_version = schema_version;
+ it->schema_state = schema_state;
it->space_id = index->def->space_id;
it->index_id = index->def->iid;
it->index = index;
@@ -443,15 +443,15 @@ int
iterator_next(struct iterator *it, struct tuple **ret)
{
assert(it->next != NULL);
- if (unlikely(it->schema_version != schema_version)) {
+ if (unlikely(it->schema_state != schema_state)) {
struct space *space = space_by_id(it->space_id);
if (space == NULL)
goto invalidate;
struct index *index = space_index(space, it->index_id);
if (index != it->index ||
- index->schema_version > it->schema_version)
+ index->schema_state > it->schema_state)
goto invalidate;
- it->schema_version = schema_version;
+ it->schema_state = schema_state;
}
return it->next(it, ret);
@@ -478,7 +478,7 @@ index_create(struct index *index, struct engine *engine,
index->vtab = vtab;
index->engine = engine;
index->def = def;
- index->schema_version = schema_version;
+ index->schema_state = schema_state;
return 0;
}
diff --git a/src/box/index.h b/src/box/index.h
index 3c478c6d6..cc9de57a5 100644
--- a/src/box/index.h
+++ b/src/box/index.h
@@ -233,8 +233,8 @@ struct iterator {
int (*next)(struct iterator *it, struct tuple **ret);
/** Destroy the iterator. */
void (*free)(struct iterator *);
- /** Schema version at the time of the last index lookup. */
- uint32_t schema_version;
+ /** Schema state at the time of the last index lookup. */
+ uint32_t schema_state;
/** ID of the space the iterator is for. */
uint32_t space_id;
/** ID of the index the iterator is for. */
@@ -242,7 +242,7 @@ struct iterator {
/**
* Pointer to the index the iterator is for.
* Guaranteed to be valid only if the schema
- * version has not changed since the last lookup.
+ * state has not changed since the last lookup.
*/
struct index *index;
};
@@ -408,8 +408,8 @@ struct index {
struct engine *engine;
/* Description of a possibly multipart key. */
struct index_def *def;
- /* Schema version at the time of construction. */
- uint32_t schema_version;
+ /* Schema state at the time of construction. */
+ uint32_t schema_state;
};
/**
diff --git a/src/box/schema.cc b/src/box/schema.cc
index 1b96f978c..02b4f54ee 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -58,7 +58,15 @@ static struct mh_i32ptr_t *spaces;
static struct mh_i32ptr_t *funcs;
static struct mh_strnptr_t *funcs_by_name;
static struct mh_i32ptr_t *sequences;
+/** Public change counter. On its update clients need to fetch
+ * new space data from the instance. */
uint32_t schema_version = 0;
+/**
+ * Internal change counter. Grows faster, than the public one,
+ * because we need to remember when to update pointers to already
+ * non-existent space objects on space:truncate() operation.
+ */
+uint32_t schema_state = 0;
uint32_t dd_version_id = version_id(1, 6, 4);
struct rlist on_alter_space = RLIST_HEAD_INITIALIZER(on_alter_space);
@@ -157,7 +165,7 @@ space_cache_delete(uint32_t id)
assert(k != mh_end(spaces));
struct space *space = (struct space *)mh_i32ptr_node(spaces, k)->val;
mh_i32ptr_del(spaces, k, NULL);
- schema_version++;
+ schema_state++;
return space;
}
@@ -175,7 +183,7 @@ space_cache_replace(struct space *space)
panic_syserror("Out of memory for the data "
"dictionary cache.");
}
- schema_version++;
+ schema_state++;
return p_old ? (struct space *) p_old->val : NULL;
}
diff --git a/src/box/schema.h b/src/box/schema.h
index 56f39b3fe..dcc764aef 100644
--- a/src/box/schema.h
+++ b/src/box/schema.h
@@ -42,6 +42,7 @@ extern "C" {
#endif /* defined(__cplusplus) */
extern uint32_t schema_version;
+extern uint32_t schema_state;
/**
* Persistent version of the schema, stored in _schema["version"].
@@ -68,14 +69,14 @@ box_schema_version();
static inline struct space *
space_cache_find(uint32_t id)
{
- static uint32_t prev_schema_version = 0;
+ static uint32_t prev_schema_state = 0;
static struct space *space = NULL;
- if (prev_schema_version != schema_version)
+ if (prev_schema_state != schema_state)
space = NULL;
if (space && space->def->id == id)
return space;
if ((space = space_by_id(id))) {
- prev_schema_version = schema_version;
+ prev_schema_state = schema_state;
return space;
}
diag_set(ClientError, ER_NO_SUCH_SPACE, int2str(id));
diff --git a/src/box/sysview_index.c b/src/box/sysview_index.c
index 0bec30282..6a2667db2 100644
--- a/src/box/sysview_index.c
+++ b/src/box/sysview_index.c
@@ -66,7 +66,7 @@ sysview_iterator_next(struct iterator *iterator, struct tuple **ret)
assert(iterator->free == sysview_iterator_free);
struct sysview_iterator *it = sysview_iterator(iterator);
*ret = NULL;
- if (it->source->schema_version != schema_version)
+ if (it->source->schema_state != schema_state)
return 0; /* invalidate iterator */
struct sysview_index *index = (struct sysview_index *)iterator->index;
int rc;
diff --git a/src/box/vy_index.c b/src/box/vy_index.c
index d9160041b..1d92bebd3 100644
--- a/src/box/vy_index.c
+++ b/src/box/vy_index.c
@@ -217,7 +217,7 @@ vy_index_new(struct vy_index_env *index_env, struct vy_cache_env *cache_env,
index->mem = vy_mem_new(mem_env, *index->env->p_generation,
cmp_def, format, index->mem_format_with_colmask,
- index->upsert_format, schema_version);
+ index->upsert_format, schema_state);
if (index->mem == NULL)
goto fail_mem;
@@ -764,7 +764,7 @@ vy_index_rotate_mem(struct vy_index *index)
mem = vy_mem_new(index->mem->env, *index->env->p_generation,
index->cmp_def, index->mem_format,
index->mem_format_with_colmask,
- index->upsert_format, schema_version);
+ index->upsert_format, schema_state);
if (mem == NULL)
return -1;
diff --git a/src/box/vy_mem.c b/src/box/vy_mem.c
index 629634bbf..d7f62fbba 100644
--- a/src/box/vy_mem.c
+++ b/src/box/vy_mem.c
@@ -98,7 +98,7 @@ struct vy_mem *
vy_mem_new(struct vy_mem_env *env, int64_t generation,
const struct key_def *cmp_def, struct tuple_format *format,
struct tuple_format *format_with_colmask,
- struct tuple_format *upsert_format, uint32_t schema_version)
+ struct tuple_format *upsert_format, uint32_t schema_state)
{
struct vy_mem *index = calloc(1, sizeof(*index));
if (!index) {
@@ -111,7 +111,7 @@ vy_mem_new(struct vy_mem_env *env, int64_t generation,
index->max_lsn = -1;
index->cmp_def = cmp_def;
index->generation = generation;
- index->schema_version = schema_version;
+ index->schema_state = schema_state;
index->format = format;
tuple_format_ref(format);
index->format_with_colmask = format_with_colmask;
diff --git a/src/box/vy_mem.h b/src/box/vy_mem.h
index d60032219..6f4af3c8d 100644
--- a/src/box/vy_mem.h
+++ b/src/box/vy_mem.h
@@ -174,8 +174,8 @@ struct vy_mem {
const struct key_def *cmp_def;
/** version is initially 0 and is incremented on every write */
uint32_t version;
- /** Schema version at the time of creation. */
- uint32_t schema_version;
+ /** Schema state at the time of creation. */
+ uint32_t schema_state;
/**
* Generation of statements stored in the tree.
* Used as lsregion allocator identifier.
@@ -250,7 +250,7 @@ vy_mem_wait_pinned(struct vy_mem *mem)
* @param format_with_colmask Format for tuples, which have
* column mask.
* @param upsert_format Format for UPSERT tuples.
- * @param schema_version Schema version.
+ * @param schema_state Schema state.
* @retval new vy_mem instance on success.
* @retval NULL on error, check diag.
*/
@@ -258,7 +258,7 @@ struct vy_mem *
vy_mem_new(struct vy_mem_env *env, int64_t generation,
const struct key_def *cmp_def, struct tuple_format *format,
struct tuple_format *format_with_colmask,
- struct tuple_format *upsert_format, uint32_t schema_version);
+ struct tuple_format *upsert_format, uint32_t schema_state);
/**
* Delete in-memory level.
diff --git a/src/box/vy_tx.c b/src/box/vy_tx.c
index 01130020b..019320ca4 100644
--- a/src/box/vy_tx.c
+++ b/src/box/vy_tx.c
@@ -45,7 +45,7 @@
#include "iproto_constants.h"
#include "iterator_type.h"
#include "salad/stailq.h"
-#include "schema.h" /* schema_version */
+#include "schema.h" /* schema_state */
#include "trigger.h"
#include "trivia/util.h"
#include "tuple.h"
@@ -414,11 +414,11 @@ vy_tx_write_prepare(struct txv *v)
* In this case we need to dump the tree as is in order to
* guarantee dump consistency.
*
- * - Schema version has increased after the tree was created.
+ * - Schema state has increased after the tree was created.
* We have to seal the tree, because we don't support mixing
* statements of different formats in the same tree.
*/
- if (unlikely(index->mem->schema_version != schema_version ||
+ if (unlikely(index->mem->schema_state != schema_state ||
index->mem->generation != *index->env->p_generation)) {
if (vy_index_rotate_mem(index) != 0)
return -1;
diff --git a/test/engine/ddl.result b/test/engine/ddl.result
index c3cb1efc2..b6b6faf5b 100644
--- a/test/engine/ddl.result
+++ b/test/engine/ddl.result
@@ -388,6 +388,63 @@ s:drop()
---
...
--
+-- gh-3414: do not increase schema_version on space:truncate()
+--
+-- update schema_version on space.create()
+sch_ver = box.internal.schema_version
+---
+...
+v = sch_ver()
+---
+...
+s = box.schema.create_space('test')
+---
+...
+v + 1 == sch_ver()
+---
+- true
+...
+-- update schema_version on space:create_index()
+prim = s:create_index("primary")
+---
+...
+v + 2 == sch_ver()
+---
+- true
+...
+-- do not change schema_version on space.truncate()
+s:truncate()
+---
+...
+v + 2 == sch_ver()
+---
+- true
+...
+-- update schema_version on index.alter()
+prim:alter{name="new_primary"}
+---
+...
+v + 3 == sch_ver()
+---
+- true
+...
+-- update schema_version on index.drop()
+box.schema.index.drop(s.id, 0)
+---
+...
+v + 4 == sch_ver()
+---
+- true
+...
+-- update schema_version on space.drop()
+s:drop()
+---
+...
+v + 5 == sch_ver()
+---
+- true
+...
+--
-- gh-3229: update optionality if a space format is changed too,
-- not only when indexes are updated.
--
diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua
index 25381a3c8..e73c7dbb0 100644
--- a/test/engine/ddl.test.lua
+++ b/test/engine/ddl.test.lua
@@ -137,6 +137,30 @@ pk = s:create_index('pk')
pk.parts[1].type
s:drop()
+--
+-- gh-3414: do not increase schema_version on space:truncate()
+--
+-- update schema_version on space.create()
+sch_ver = box.internal.schema_version
+v = sch_ver()
+s = box.schema.create_space('test')
+v + 1 == sch_ver()
+-- update schema_version on space:create_index()
+prim = s:create_index("primary")
+v + 2 == sch_ver()
+-- do not change schema_version on space.truncate()
+s:truncate()
+v + 2 == sch_ver()
+-- update schema_version on index.alter()
+prim:alter{name="new_primary"}
+v + 3 == sch_ver()
+-- update schema_version on index.drop()
+box.schema.index.drop(s.id, 0)
+v + 4 == sch_ver()
+-- update schema_version on space.drop()
+s:drop()
+v + 5 == sch_ver()
+
--
-- gh-3229: update optionality if a space format is changed too,
-- not only when indexes are updated.
diff --git a/test/unit/vy_point_lookup.c b/test/unit/vy_point_lookup.c
index 963329c9f..914833e02 100644
--- a/test/unit/vy_point_lookup.c
+++ b/test/unit/vy_point_lookup.c
@@ -12,6 +12,7 @@
#include "identifier.h"
uint32_t schema_version;
+uint32_t schema_state;
static int
write_run(struct vy_run *run, const char *dir_name,
--
2.15.2 (Apple Git-101.1)
More information about the Tarantool-patches
mailing list