From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 2EF67469719 for ; Fri, 20 Mar 2020 15:32:18 +0300 (MSK) From: Nikita Pettik Date: Fri, 20 Mar 2020 15:32:15 +0300 Message-Id: <59ece61283f53c6d2772b81ffc8a783873ef4b0b.1584703666.git.korablev@tarantool.org> Subject: [Tarantool-patches] [PATCH] vinyl: update mem ptr in vy_build_insert_tuple() after yield List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org 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