* [PATCH] memtx: add yields during index build
@ 2019-05-22 16:11 Serge Petrenko
2019-05-22 16:28 ` Vladimir Davydov
0 siblings, 1 reply; 4+ messages in thread
From: Serge Petrenko @ 2019-05-22 16:11 UTC (permalink / raw)
To: tarantool-patches; +Cc: vdavydov.dev, Serge Petrenko
Memtx index build used to stall event loop for all the build period.
Add occasional yields so that the loop is not blocked for too long.
Closes #3976
---
https://github.com/tarantool/tarantool/issues/3976
https://github.com/tarantool/tarantool/tree/sp/gh-3976-background-index-build
src/box/memtx_space.c | 13 +++++
test/box/memtx_background_index_build.result | 55 +++++++++++++++++++
.../box/memtx_background_index_build.test.lua | 32 +++++++++++
test/box/suite.ini | 2 +-
4 files changed, 101 insertions(+), 1 deletion(-)
create mode 100644 test/box/memtx_background_index_build.result
create mode 100644 test/box/memtx_background_index_build.test.lua
diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index 5ddb4f7ee..b90e2707e 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -874,6 +874,15 @@ static int
memtx_space_build_index(struct space *src_space, struct index *new_index,
struct tuple_format *new_format)
{
+ /*
+ * Yield every 1K tuples.
+ * In debug mode yield more often for testing purposes.
+ */
+#ifdef NDEBUG
+ enum { YIELD_LOOPS = 1000 };
+#else
+ enum { YIELD_LOOPS = 10 };
+#endif
/**
* If it's a secondary key, and we're not building them
* yet (i.e. it's snapshot recovery for memtx), do nothing.
@@ -909,6 +918,7 @@ memtx_space_build_index(struct space *src_space, struct index *new_index,
/* Build the new index. */
int rc;
struct tuple *tuple;
+ size_t count = 0;
while ((rc = iterator_next(it, &tuple)) == 0 && tuple != NULL) {
/*
* Check that the tuple is OK according to the
@@ -933,6 +943,9 @@ memtx_space_build_index(struct space *src_space, struct index *new_index,
*/
if (new_index->def->iid == 0)
tuple_ref(tuple);
+ if (++count % YIELD_LOOPS == 0) {
+ fiber_sleep(0);
+ }
}
iterator_delete(it);
return rc;
diff --git a/test/box/memtx_background_index_build.result b/test/box/memtx_background_index_build.result
new file mode 100644
index 000000000..332595bc2
--- /dev/null
+++ b/test/box/memtx_background_index_build.result
@@ -0,0 +1,55 @@
+env = require('test_run')
+---
+...
+test_run = env.new()
+---
+...
+fiber = require('fiber')
+---
+...
+num_iters = 0
+---
+...
+test_run:cmd('setopt delimiter ";"')
+---
+- true
+...
+function f()
+ while true do
+ num_iters = num_iters + 1
+ fiber.yield()
+ fiber.testcancel()
+ end
+end;
+---
+...
+test_run:cmd('setopt delimiter ""');
+---
+- true
+...
+_ = box.schema.space.create('test')
+---
+...
+_ = box.space.test:create_index('pk')
+---
+...
+for i = 1,1000 do box.space.test:insert{i} end
+---
+...
+fib = fiber.new(f) _ = box.space.test:create_index('sk') fiber.cancel(fib) fiber.yield()
+---
+...
+fib
+---
+- the fiber is dead
+...
+-- index build in debug mode should yield every 10 iterations.
+-- This means we will have at least 100 event loop iterations
+-- during index build.
+num_iters >= 100 or num_iters
+---
+- true
+...
+box.space.test:drop()
+---
+...
diff --git a/test/box/memtx_background_index_build.test.lua b/test/box/memtx_background_index_build.test.lua
new file mode 100644
index 000000000..7a5711cf6
--- /dev/null
+++ b/test/box/memtx_background_index_build.test.lua
@@ -0,0 +1,32 @@
+env = require('test_run')
+test_run = env.new()
+
+fiber = require('fiber')
+
+num_iters = 0
+
+test_run:cmd('setopt delimiter ";"')
+
+function f()
+ while true do
+ num_iters = num_iters + 1
+ fiber.yield()
+ fiber.testcancel()
+ end
+end;
+test_run:cmd('setopt delimiter ""');
+
+_ = box.schema.space.create('test')
+_ = box.space.test:create_index('pk')
+
+for i = 1,1000 do box.space.test:insert{i} end
+
+fib = fiber.new(f) _ = box.space.test:create_index('sk') fiber.cancel(fib) fiber.yield()
+fib
+
+-- index build in debug mode should yield every 10 iterations.
+-- This means we will have at least 100 event loop iterations
+-- during index build.
+num_iters >= 100 or num_iters
+
+box.space.test:drop()
diff --git a/test/box/suite.ini b/test/box/suite.ini
index c7b75c173..5283d2031 100644
--- a/test/box/suite.ini
+++ b/test/box/suite.ini
@@ -3,7 +3,7 @@ core = tarantool
description = Database tests
script = box.lua
disabled = rtree_errinj.test.lua tuple_bench.test.lua
-release_disabled = errinj.test.lua errinj_index.test.lua rtree_errinj.test.lua upsert_errinj.test.lua iproto_stress.test.lua
+release_disabled = errinj.test.lua errinj_index.test.lua rtree_errinj.test.lua upsert_errinj.test.lua iproto_stress.test.lua memtx_background_index_build.test.lua
lua_libs = lua/fifo.lua lua/utils.lua lua/bitset.lua lua/index_random_test.lua lua/push.lua lua/identifier.lua
use_unix_sockets = True
is_parallel = True
--
2.20.1 (Apple Git-117)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] memtx: add yields during index build
2019-05-22 16:11 [PATCH] memtx: add yields during index build Serge Petrenko
@ 2019-05-22 16:28 ` Vladimir Davydov
2019-05-22 19:02 ` [tarantool-patches] " Konstantin Osipov
2019-05-23 14:13 ` [tarantool-patches] " Serge Petrenko
0 siblings, 2 replies; 4+ messages in thread
From: Vladimir Davydov @ 2019-05-22 16:28 UTC (permalink / raw)
To: Serge Petrenko; +Cc: tarantool-patches
On Wed, May 22, 2019 at 07:11:42PM +0300, Serge Petrenko wrote:
> Memtx index build used to stall event loop for all the build period.
> Add occasional yields so that the loop is not blocked for too long.
>
> Closes #3976
> ---
> https://github.com/tarantool/tarantool/issues/3976
> https://github.com/tarantool/tarantool/tree/sp/gh-3976-background-index-build
>
> src/box/memtx_space.c | 13 +++++
> test/box/memtx_background_index_build.result | 55 +++++++++++++++++++
> .../box/memtx_background_index_build.test.lua | 32 +++++++++++
> test/box/suite.ini | 2 +-
> 4 files changed, 101 insertions(+), 1 deletion(-)
> create mode 100644 test/box/memtx_background_index_build.result
> create mode 100644 test/box/memtx_background_index_build.test.lua
>
> diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
> index 5ddb4f7ee..b90e2707e 100644
> --- a/src/box/memtx_space.c
> +++ b/src/box/memtx_space.c
> @@ -874,6 +874,15 @@ static int
> memtx_space_build_index(struct space *src_space, struct index *new_index,
> struct tuple_format *new_format)
> {
> + /*
> + * Yield every 1K tuples.
> + * In debug mode yield more often for testing purposes.
> + */
> +#ifdef NDEBUG
> + enum { YIELD_LOOPS = 1000 };
> +#else
> + enum { YIELD_LOOPS = 10 };
> +#endif
> /**
> * If it's a secondary key, and we're not building them
> * yet (i.e. it's snapshot recovery for memtx), do nothing.
> @@ -909,6 +918,7 @@ memtx_space_build_index(struct space *src_space, struct index *new_index,
> /* Build the new index. */
> int rc;
> struct tuple *tuple;
> + size_t count = 0;
> while ((rc = iterator_next(it, &tuple)) == 0 && tuple != NULL) {
> /*
> * Check that the tuple is OK according to the
> @@ -933,6 +943,9 @@ memtx_space_build_index(struct space *src_space, struct index *new_index,
> */
> if (new_index->def->iid == 0)
> tuple_ref(tuple);
> + if (++count % YIELD_LOOPS == 0) {
> + fiber_sleep(0);
> + }
This isn't enough as tuples may be inserted into / deleted from the
space while DDL is in progress. We need to propagate these changes
to the new index using on_replace trigger. Take a look at how vinyl
handles it.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [tarantool-patches] Re: [PATCH] memtx: add yields during index build
2019-05-22 16:28 ` Vladimir Davydov
@ 2019-05-22 19:02 ` Konstantin Osipov
2019-05-23 14:13 ` [tarantool-patches] " Serge Petrenko
1 sibling, 0 replies; 4+ messages in thread
From: Konstantin Osipov @ 2019-05-22 19:02 UTC (permalink / raw)
To: tarantool-patches; +Cc: Serge Petrenko
* Vladimir Davydov <vdavydov.dev@gmail.com> [19/05/22 21:45]:
> This isn't enough as tuples may be inserted into / deleted from the
> space while DDL is in progress. We need to propagate these changes
> to the new index using on_replace trigger. Take a look at how vinyl
> handles it.
+
--
Konstantin Osipov, Moscow, Russia
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [tarantool-patches] [PATCH] memtx: add yields during index build
2019-05-22 16:28 ` Vladimir Davydov
2019-05-22 19:02 ` [tarantool-patches] " Konstantin Osipov
@ 2019-05-23 14:13 ` Serge Petrenko
1 sibling, 0 replies; 4+ messages in thread
From: Serge Petrenko @ 2019-05-23 14:13 UTC (permalink / raw)
To: Vladimir Davydov; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 2496 bytes --]
> 22 мая 2019 г., в 19:28, Vladimir Davydov <vdavydov.dev@gmail.com> написал(а):
>
> On Wed, May 22, 2019 at 07:11:42PM +0300, Serge Petrenko wrote:
>> Memtx index build used to stall event loop for all the build period.
>> Add occasional yields so that the loop is not blocked for too long.
>>
>> Closes #3976
>> ---
>> https://github.com/tarantool/tarantool/issues/3976
>> https://github.com/tarantool/tarantool/tree/sp/gh-3976-background-index-build
>>
>> src/box/memtx_space.c | 13 +++++
>> test/box/memtx_background_index_build.result | 55 +++++++++++++++++++
>> .../box/memtx_background_index_build.test.lua | 32 +++++++++++
>> test/box/suite.ini | 2 +-
>> 4 files changed, 101 insertions(+), 1 deletion(-)
>> create mode 100644 test/box/memtx_background_index_build.result
>> create mode 100644 test/box/memtx_background_index_build.test.lua
>>
>> diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
>> index 5ddb4f7ee..b90e2707e 100644
>> --- a/src/box/memtx_space.c
>> +++ b/src/box/memtx_space.c
>> @@ -874,6 +874,15 @@ static int
>> memtx_space_build_index(struct space *src_space, struct index *new_index,
>> struct tuple_format *new_format)
>> {
>> + /*
>> + * Yield every 1K tuples.
>> + * In debug mode yield more often for testing purposes.
>> + */
>> +#ifdef NDEBUG
>> + enum { YIELD_LOOPS = 1000 };
>> +#else
>> + enum { YIELD_LOOPS = 10 };
>> +#endif
>> /**
>> * If it's a secondary key, and we're not building them
>> * yet (i.e. it's snapshot recovery for memtx), do nothing.
>> @@ -909,6 +918,7 @@ memtx_space_build_index(struct space *src_space, struct index *new_index,
>> /* Build the new index. */
>> int rc;
>> struct tuple *tuple;
>> + size_t count = 0;
>> while ((rc = iterator_next(it, &tuple)) == 0 && tuple != NULL) {
>> /*
>> * Check that the tuple is OK according to the
>> @@ -933,6 +943,9 @@ memtx_space_build_index(struct space *src_space, struct index *new_index,
>> */
>> if (new_index->def->iid == 0)
>> tuple_ref(tuple);
>> + if (++count % YIELD_LOOPS == 0) {
>> + fiber_sleep(0);
>> + }
>
> This isn't enough as tuples may be inserted into / deleted from the
> space while DDL is in progress. We need to propagate these changes
> to the new index using on_replace trigger. Take a look at how vinyl
> handles it.
Thank you for pointing this out! Please see v2 in the mailing list.
[-- Attachment #2: Type: text/html, Size: 10187 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-23 14:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-22 16:11 [PATCH] memtx: add yields during index build Serge Petrenko
2019-05-22 16:28 ` Vladimir Davydov
2019-05-22 19:02 ` [tarantool-patches] " Konstantin Osipov
2019-05-23 14:13 ` [tarantool-patches] " Serge Petrenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox