Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH] vinyl: update mem ptr in vy_build_insert_tuple() after yield
@ 2020-03-20 12:32 Nikita Pettik
  2020-03-20 12:42 ` Nikita Pettik
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Nikita Pettik @ 2020-03-20 12:32 UTC (permalink / raw)
  To: tarantool-patches

vy_build_insert_tuple() processes insertion into secondary indexes being
created. It contains yield points during which in-memory level of LSM
tree may change (for example rotate owing to triggered dump). So after
yield point it is required to fetch from LSM struct pointer to mem again
to operate on valid metadata. This patch updates pointer to mem after
mentioned yield point.

Closes #4810
---
Branch: https://github.com/tarantool/tarantool/tree/np/gh-4810-dump-during-index-build
Issue: https://github.com/tarantool/tarantool/issues/4810

@ChangeLog:
- Fixed assertion fault due to triggered dump process during secondary
  index build (gh-4810).

 src/box/vinyl.c                               |  10 ++
 .../gh-4810-dump-during-index-build.result    | 150 ++++++++++++++++++
 .../gh-4810-dump-during-index-build.test.lua  |  74 +++++++++
 3 files changed, 234 insertions(+)
 create mode 100644 test/vinyl/gh-4810-dump-during-index-build.result
 create mode 100644 test/vinyl/gh-4810-dump-during-index-build.test.lua

diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 252e4e774..55a4c8fb7 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -4086,6 +4086,16 @@ vy_build_insert_tuple(struct vy_env *env, struct vy_lsm *lsm,
 		tuple_unref(stmt);
 		return -1;
 	}
+	/*
+	 * Despite the fact that we protected mem from being
+	 * dumped, its generation still may bump due to rotation
+	 * in vy_tx_write_prepare() (insertions during index build
+	 * are still handled by on_replace_trigger). This may happen
+	 * if dump process is triggered (vy_scheduler_trigger_dump()).
+	 * Hence, to get right mem (during mem rotation it becomes
+	 * sealed i.e. read-only) we should fetch it from lsm again.
+	 */
+	mem = lsm->mem;
 
 	/* Insert the new tuple into the in-memory index. */
 	size_t mem_used_before = lsregion_used(&env->mem_env.allocator);
diff --git a/test/vinyl/gh-4810-dump-during-index-build.result b/test/vinyl/gh-4810-dump-during-index-build.result
new file mode 100644
index 000000000..f55c44a1f
--- /dev/null
+++ b/test/vinyl/gh-4810-dump-during-index-build.result
@@ -0,0 +1,150 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+
+test_run:cmd("create server test with script='vinyl/low_quota.lua'")
+ | ---
+ | - true
+ | ...
+test_run:cmd("start server test with args='13421772'")
+ | ---
+ | - true
+ | ...
+test_run:cmd('switch test')
+ | ---
+ | - true
+ | ...
+
+fiber = require 'fiber'
+ | ---
+ | ...
+
+math.randomseed(os.time())
+ | ---
+ | ...
+
+s = box.schema.space.create('test', {engine = 'vinyl'})
+ | ---
+ | ...
+_ = s:create_index('pk', {parts = {1, 'unsigned'}})
+ | ---
+ | ...
+
+--
+-- Purpose of this test is to trigger dump during secondary index build.
+-- It is worth noting that dump must be triggered by exceeding memory
+-- quota and not by invoking box.snapshot() since checkpoint process
+-- is locked by DDL latch.
+--
+
+MAX_KEY = 1000000
+ | ---
+ | ...
+MAX_VAL = 1000000
+ | ---
+ | ...
+PADDING = string.rep('x', 100)
+ | ---
+ | ...
+
+test_run:cmd("setopt delimiter ';'")
+ | ---
+ | - true
+ | ...
+
+function gen_insert()
+    pcall(s.insert, s, {math.random(MAX_KEY), math.random(MAX_VAL),
+                        math.random(MAX_VAL), math.random(MAX_VAL), PADDING})
+end;
+ | ---
+ | ...
+
+function fill_L0_without_dump()
+    local dump_watermark = box.cfg.vinyl_memory / 2
+    while box.stat.vinyl().memory.level0 < dump_watermark do
+        gen_insert()
+    end
+end;
+ | ---
+ | ...
+
+fill_L0_without_dump();
+ | ---
+ | ...
+assert(box.stat.vinyl().scheduler.dump_count == 0);
+ | ---
+ | - true
+ | ...
+
+function insert_loop()
+    while not stop do
+        gen_insert()
+    end
+    ch:put(true)
+end;
+ | ---
+ | ...
+
+function idx_build()
+    _ = s:create_index('i1', {unique = true, parts = {2, 'unsigned', 3, 'unsigned'}})
+    ch:put(true)
+end;
+ | ---
+ | ...
+
+stop = false;
+ | ---
+ | ...
+ch = fiber.channel(2);
+ | ---
+ | ...
+
+_ = fiber.create(idx_build);
+ | ---
+ | ...
+_ = fiber.create(insert_loop);
+ | ---
+ | ...
+
+fiber.sleep(3);
+ | ---
+ | ...
+
+stop = true;
+ | ---
+ | ...
+for i = 1, ch:size() do
+    ch:get()
+end;
+ | ---
+ | ...
+
+test_run:cmd("setopt delimiter ''");
+ | ---
+ | - true
+ | ...
+assert(box.stat.vinyl().scheduler.dump_count > 0)
+ | ---
+ | - true
+ | ...
+assert(s.index.i1 ~= nil)
+ | ---
+ | - true
+ | ...
+s:drop()
+ | ---
+ | ...
+
+test_run:cmd('switch default')
+ | ---
+ | - true
+ | ...
+test_run:cmd("stop server test")
+ | ---
+ | - true
+ | ...
+test_run:cmd("cleanup server test")
+ | ---
+ | - true
+ | ...
diff --git a/test/vinyl/gh-4810-dump-during-index-build.test.lua b/test/vinyl/gh-4810-dump-during-index-build.test.lua
new file mode 100644
index 000000000..7b7026498
--- /dev/null
+++ b/test/vinyl/gh-4810-dump-during-index-build.test.lua
@@ -0,0 +1,74 @@
+test_run = require('test_run').new()
+
+test_run:cmd("create server test with script='vinyl/low_quota.lua'")
+test_run:cmd("start server test with args='13421772'")
+test_run:cmd('switch test')
+
+fiber = require 'fiber'
+
+math.randomseed(os.time())
+
+s = box.schema.space.create('test', {engine = 'vinyl'})
+_ = s:create_index('pk', {parts = {1, 'unsigned'}})
+
+--
+-- Purpose of this test is to trigger dump during secondary index build.
+-- It is worth noting that dump must be triggered by exceeding memory
+-- quota and not by invoking box.snapshot() since checkpoint process
+-- is locked by DDL latch.
+--
+
+MAX_KEY = 1000000
+MAX_VAL = 1000000
+PADDING = string.rep('x', 100)
+
+test_run:cmd("setopt delimiter ';'")
+
+function gen_insert()
+    pcall(s.insert, s, {math.random(MAX_KEY), math.random(MAX_VAL),
+                        math.random(MAX_VAL), math.random(MAX_VAL), PADDING})
+end;
+
+function fill_L0_without_dump()
+    local dump_watermark = box.cfg.vinyl_memory / 2
+    while box.stat.vinyl().memory.level0 < dump_watermark do
+        gen_insert()
+    end
+end;
+
+fill_L0_without_dump();
+assert(box.stat.vinyl().scheduler.dump_count == 0);
+
+function insert_loop()
+    while not stop do
+        gen_insert()
+    end
+    ch:put(true)
+end;
+
+function idx_build()
+    _ = s:create_index('i1', {unique = true, parts = {2, 'unsigned', 3, 'unsigned'}})
+    ch:put(true)
+end;
+
+stop = false;
+ch = fiber.channel(2);
+
+_ = fiber.create(idx_build);
+_ = fiber.create(insert_loop);
+
+fiber.sleep(3);
+
+stop = true;
+for i = 1, ch:size() do
+    ch:get()
+end;
+
+test_run:cmd("setopt delimiter ''");
+assert(box.stat.vinyl().scheduler.dump_count > 0)
+assert(s.index.i1 ~= nil)
+s:drop()
+
+test_run:cmd('switch default')
+test_run:cmd("stop server test")
+test_run:cmd("cleanup server test")
-- 
2.17.1

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-03-20 19:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-20 12:32 [Tarantool-patches] [PATCH] vinyl: update mem ptr in vy_build_insert_tuple() after yield Nikita Pettik
2020-03-20 12:42 ` Nikita Pettik
2020-03-20 13:33 ` Konstantin Osipov
2020-03-20 15:06   ` Nikita Pettik
2020-03-20 15:19     ` Kirill Yukhin
2020-03-20 17:45       ` Konstantin Osipov
2020-03-20 17:40     ` Konstantin Osipov
2020-03-20 18:24       ` Nikita Pettik
2020-03-20 19:40         ` Konstantin Osipov
2020-03-20 15:23 ` Kirill Yukhin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox