[patches] [PATCH 4/4] vinyl: on space alter update key definitions in indexes
v.shpilevoy at tarantool.org
v.shpilevoy at tarantool.org
Mon Mar 12 18:17:35 MSK 2018
> 12 марта 2018 г., в 17:54, v.shpilevoy at tarantool.org написал(а):
>
>
>>
>>>
>>> Anyway, this isn't enough. Apart from worker threads, there are also
>>> reader fibers, which access key_def too. Referencing key_def for them is
>>> likely to turn the code into even a bigger mess than it is with this
>>> patch...
>>
>> I can not find places in a code, where key_def has no 'index->' before it, except workers (write_iterator), mems, ranges and cache. For workers I moved key_def into struct task. For other places I update key_def in commit_alter(). Where key_def is used in Tx fibers instead of different threads via index->key_def, there is no problems.
>
> Oh, I see. Vy_run_iterator has key_def too. I will fix it.
Fixed on the branch:
diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index f8d96f33e..7b89c6828 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -1405,15 +1405,16 @@ vy_run_iterator_open(struct vy_run_iterator *itr,
struct vy_run_iterator_stat *stat,
struct vy_slice *slice, enum iterator_type iterator_type,
const struct tuple *key, const struct vy_read_view **rv,
- const struct key_def *cmp_def,
- const struct key_def *key_def,
+ struct key_def *cmp_def, struct key_def *key_def,
struct tuple_format *format,
struct tuple_format *upsert_format,
bool is_primary)
{
itr->stat = stat;
itr->cmp_def = cmp_def;
+ key_def_ref(cmp_def);
itr->key_def = key_def;
+ key_def_ref(key_def);
itr->format = format;
itr->upsert_format = upsert_format;
itr->is_primary = is_primary;
@@ -1553,6 +1554,8 @@ vy_run_iterator_skip(struct vy_run_iterator *itr,
void
vy_run_iterator_close(struct vy_run_iterator *itr)
{
+ key_def_unref(itr->key_def);
+ key_def_unref(itr->cmp_def);
vy_run_iterator_stop(itr);
TRASH(itr);
}
@@ -2591,6 +2594,7 @@ vy_slice_stream_close(struct vy_stmt_stream *virt_stream)
tuple_unref(stream->tuple);
stream->tuple = NULL;
}
+ key_def_unref(stream->cmp_def);
}
static const struct vy_stmt_stream_iface vy_slice_stream_iface = {
@@ -2602,8 +2606,8 @@ static const struct vy_stmt_stream_iface vy_slice_stream_iface = {
void
vy_slice_stream_open(struct vy_slice_stream *stream, struct vy_slice *slice,
- const struct key_def *cmp_def, struct tuple_format *format,
- struct tuple_format *upsert_format, bool is_primary)
+ struct key_def *cmp_def, struct tuple_format *format,
+ struct tuple_format *upsert_format, bool is_primary)
{
stream->base.iface = &vy_slice_stream_iface;
@@ -2614,6 +2618,7 @@ vy_slice_stream_open(struct vy_slice_stream *stream, struct vy_slice *slice,
stream->slice = slice;
stream->cmp_def = cmp_def;
+ key_def_ref(cmp_def);
stream->format = format;
stream->upsert_format = upsert_format;
stream->is_primary = is_primary;
diff --git a/src/box/vy_run.h b/src/box/vy_run.h
index 6973ee2d3..1857fbaeb 100644
--- a/src/box/vy_run.h
+++ b/src/box/vy_run.h
@@ -215,9 +215,9 @@ struct vy_run_iterator {
/* Members needed for memory allocation and disk access */
/** Index key definition used for storing statements on disk. */
- const struct key_def *cmp_def;
+ struct key_def *cmp_def;
/** Index key definition defined by the user. */
- const struct key_def *key_def;
+ struct key_def *key_def;
/**
* Format ot allocate REPLACE and DELETE tuples read from
* pages.
@@ -492,8 +492,7 @@ vy_run_iterator_open(struct vy_run_iterator *itr,
struct vy_run_iterator_stat *stat,
struct vy_slice *slice, enum iterator_type iterator_type,
const struct tuple *key, const struct vy_read_view **rv,
- const struct key_def *cmp_def,
- const struct key_def *key_def,
+ struct key_def *cmp_def, struct key_def *key_def,
struct tuple_format *format,
struct tuple_format *upsert_format,
bool is_primary);
@@ -551,7 +550,7 @@ struct vy_slice_stream {
* Key def for comparing with slice boundaries,
* includes secondary key parts.
*/
- const struct key_def *cmp_def;
+ struct key_def *cmp_def;
/** Format for allocating REPLACE and DELETE tuples read from pages. */
struct tuple_format *format;
/** Same as format, but for UPSERT tuples. */
@@ -565,7 +564,7 @@ struct vy_slice_stream {
*/
void
vy_slice_stream_open(struct vy_slice_stream *stream, struct vy_slice *slice,
- const struct key_def *cmp_def, struct tuple_format *format,
+ struct key_def *cmp_def, struct tuple_format *format,
struct tuple_format *upsert_format, bool is_primary);
/**
diff --git a/src/box/vy_write_iterator.c b/src/box/vy_write_iterator.c
index c2023c2c9..a7aa63e22 100644
--- a/src/box/vy_write_iterator.c
+++ b/src/box/vy_write_iterator.c
@@ -168,7 +168,7 @@ struct vy_write_iterator {
/* A heap to order the sources, newest LSN at heap top. */
heap_t src_heap;
/** Index key definition used to store statements on disk. */
- const struct key_def *cmp_def;
+ struct key_def *cmp_def;
/** Format to allocate new REPLACE and DELETE tuples from vy_run */
struct tuple_format *format;
/** Same as format, but for UPSERT tuples. */
@@ -330,7 +330,7 @@ static const struct vy_stmt_stream_iface vy_slice_stream_iface;
* @return the iterator or NULL on error (diag is set).
*/
struct vy_stmt_stream *
-vy_write_iterator_new(const struct key_def *cmp_def, struct tuple_format *format,
+vy_write_iterator_new(struct key_def *cmp_def, struct tuple_format *format,
struct tuple_format *upsert_format, bool is_primary,
bool is_last_level, struct rlist *read_views)
{
@@ -363,6 +363,7 @@ vy_write_iterator_new(const struct key_def *cmp_def, struct tuple_format *format
vy_source_heap_create(&stream->src_heap);
rlist_create(&stream->src_list);
stream->cmp_def = cmp_def;
+ key_def_ref(cmp_def);
stream->format = format;
tuple_format_ref(stream->format);
stream->upsert_format = upsert_format;
@@ -416,6 +417,7 @@ vy_write_iterator_close(struct vy_stmt_stream *vstream)
vy_write_iterator_stop(vstream);
tuple_format_unref(stream->format);
tuple_format_unref(stream->upsert_format);
+ key_def_unref(stream->cmp_def);
free(stream);
}
diff --git a/src/box/vy_write_iterator.h b/src/box/vy_write_iterator.h
index fd8f214d1..5c5ec88e8 100644
--- a/src/box/vy_write_iterator.h
+++ b/src/box/vy_write_iterator.h
@@ -231,7 +231,7 @@ struct vy_slice;
* @return the iterator or NULL on error (diag is set).
*/
struct vy_stmt_stream *
-vy_write_iterator_new(const struct key_def *cmp_def, struct tuple_format *format,
+vy_write_iterator_new(struct key_def *cmp_def, struct tuple_format *format,
struct tuple_format *upsert_format, bool is_primary,
bool is_last_level, struct rlist *read_views);
diff --git a/test/engine/ddl.result b/test/engine/ddl.result
index c3cb1efc2..fc36997fe 100644
--- a/test/engine/ddl.result
+++ b/test/engine/ddl.result
@@ -391,7 +391,7 @@ s:drop()
-- gh-3229: update optionality if a space format is changed too,
-- not only when indexes are updated.
--
-box.cfg{}
+utils = require('utils')
---
...
s = box.schema.create_space('test', {engine = engine})
@@ -422,9 +422,40 @@ s:replace{2, 3, 4}
---
- [2, 3, 4]
...
+s:replace{100, 200, 300}
+---
+- [100, 200, 300]
+...
+s:replace{300, 400, 500}
+---
+- [300, 400, 500]
+...
+s:replace{400, 500, 600}
+---
+- [400, 500, 600]
+...
+-- Do snapshot for Vinyl to test run_iterator local key_def is
+-- valid after alter().
+box.snapshot()
+---
+- ok
+...
+iter = utils.create_iterator(pk)
+---
+...
+iter.next()
+---
+- [2, 3, 4]
+...
s:format({})
---
...
+iter.iterate_over()
+---
+- 0: [100, 200, 300]
+ 1: [300, 400, 500]
+ 2: [400, 500, 600]
+...
s:insert({1})
---
- [1]
@@ -450,6 +481,9 @@ s:select({})
- [2, 3, 4]
- [3, 4]
- [4, 5]
+ - [100, 200, 300]
+ - [300, 400, 500]
+ - [400, 500, 600]
...
pk:get({4})
---
diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua
index 25381a3c8..753f7265a 100644
--- a/test/engine/ddl.test.lua
+++ b/test/engine/ddl.test.lua
@@ -141,7 +141,7 @@ s:drop()
-- gh-3229: update optionality if a space format is changed too,
-- not only when indexes are updated.
--
-box.cfg{}
+utils = require('utils')
s = box.schema.create_space('test', {engine = engine})
format = {}
format[1] = {'field1', 'unsigned'}
@@ -151,7 +151,16 @@ s:format(format)
pk = s:create_index('pk')
sk = s:create_index('sk', {parts = {{2, 'unsigned', is_nullable = true}}})
s:replace{2, 3, 4}
+s:replace{100, 200, 300}
+s:replace{300, 400, 500}
+s:replace{400, 500, 600}
+-- Do snapshot for Vinyl to test run_iterator local key_def is
+-- valid after alter().
+box.snapshot()
+iter = utils.create_iterator(pk)
+iter.next()
s:format({})
+iter.iterate_over()
s:insert({1})
s:insert({4, 5})
s:insert({3, 4})
>
More information about the Tarantool-patches
mailing list