From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH 5/6] vinyl: don't yield while logging index creation Date: Sun, 30 Jun 2019 22:40:18 +0300 Message-Id: <125bf3791e1d9fdf00720d0b7e70f818c05000c8.1561922496.git.vdavydov.dev@gmail.com> In-Reply-To: References: In-Reply-To: References: To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: Currently, we always log a vinyl index creation in the vylog file synchronously, i.e. wait for the write to complete successfully. This makes any index creation a yielding operation, even if the target space is empty. To implement transactional DDL for non-yielding statements, we need to eliminate yields in this case. We can do that by simply using vy_log_try_commit() instead of vy_log_commit() for logging index creation, because we can handle a missing VY_LOG_PREPARE_INDEX record during recovery - the code was left since before commit dd0827ba7948 ("vinyl: log new index before WAL write on DDL") which split index creation into PREPARE and COMMIT stages so all we need to do is slightly modify the test. The reason why I'm doing this now, in the series removing the schema lock, is that removal of the schema lock without making space truncation non-yielding (remember space truncation basically drops and recreates all indexes) may result in a failure while executing space.truncate() from concurrent fibers, which is rather unexpected. In particular, this is checked by engine/truncate.test.lua. So to prevent the test failure once the schema lock is removed (see the next patch), let's make empty index creation non-yielding right now. --- src/box/vy_lsm.c | 3 +-- test/unit/vy_log_stub.c | 3 +++ test/vinyl/errinj_vylog.result | 23 +++++++++++++++++++---- test/vinyl/errinj_vylog.test.lua | 10 ++++++++-- 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c index c4bcc74c..3e134cc1 100644 --- a/src/box/vy_lsm.c +++ b/src/box/vy_lsm.c @@ -339,8 +339,7 @@ vy_lsm_create(struct vy_lsm *lsm) vy_log_prepare_lsm(id, lsm->space_id, lsm->index_id, lsm->group_id, lsm->key_def); vy_log_insert_range(id, range->id, NULL, NULL); - if (vy_log_tx_commit() < 0) - return -1; + vy_log_tx_try_commit(); /* Assign the id. */ assert(lsm->id < 0); diff --git a/test/unit/vy_log_stub.c b/test/unit/vy_log_stub.c index d4668408..be73adc7 100644 --- a/test/unit/vy_log_stub.c +++ b/test/unit/vy_log_stub.c @@ -42,6 +42,9 @@ vy_log_next_id(void) void vy_log_tx_begin(void) {} +void +vy_log_tx_try_commit(void) {} + int vy_log_tx_commit(void) { diff --git a/test/vinyl/errinj_vylog.result b/test/vinyl/errinj_vylog.result index 06cc6818..8fb5deda 100644 --- a/test/vinyl/errinj_vylog.result +++ b/test/vinyl/errinj_vylog.result @@ -161,10 +161,24 @@ s2.index.pk:drop() --- ... -- pending records must not be rolled back on error -s2:create_index('pk') -- error +SCHED_TIMEOUT = 0.05 +--- +... +box.error.injection.set('ERRINJ_VY_SCHED_TIMEOUT', SCHED_TIMEOUT) +--- +- ok +... +box.snapshot() -- error --- - error: Error injection 'vinyl log flush' ... +fiber.sleep(2 * SCHED_TIMEOUT) +--- +... +box.error.injection.set('ERRINJ_VY_SCHED_TIMEOUT', 0) +--- +- ok +... box.error.injection.set('ERRINJ_VY_LOG_FLUSH', false); --- - ok @@ -260,6 +274,10 @@ _ = s1:insert{333, 'ccc'} s2.index.pk:drop() --- ... +-- VY_LOG_PREPARE_LSM missing +_ = s2:create_index('pk') +--- +... test_run:cmd('restart server default') s1 = box.space.test1 --- @@ -270,9 +288,6 @@ s2 = box.space.test2 _ = s1:insert{444, 'ddd'} --- ... -_ = s2:create_index('pk') ---- -... _ = s2:insert{555, 'eee'} --- ... diff --git a/test/vinyl/errinj_vylog.test.lua b/test/vinyl/errinj_vylog.test.lua index 2936f879..5ec58682 100644 --- a/test/vinyl/errinj_vylog.test.lua +++ b/test/vinyl/errinj_vylog.test.lua @@ -79,7 +79,11 @@ _ = s1:insert{3, 'c'} s2.index.pk:drop() -- pending records must not be rolled back on error -s2:create_index('pk') -- error +SCHED_TIMEOUT = 0.05 +box.error.injection.set('ERRINJ_VY_SCHED_TIMEOUT', SCHED_TIMEOUT) +box.snapshot() -- error +fiber.sleep(2 * SCHED_TIMEOUT) +box.error.injection.set('ERRINJ_VY_SCHED_TIMEOUT', 0) box.error.injection.set('ERRINJ_VY_LOG_FLUSH', false); @@ -126,13 +130,15 @@ _ = s1:insert{333, 'ccc'} -- VY_LOG_DROP_LSM missing s2.index.pk:drop() +-- VY_LOG_PREPARE_LSM missing +_ = s2:create_index('pk') + test_run:cmd('restart server default') s1 = box.space.test1 s2 = box.space.test2 _ = s1:insert{444, 'ddd'} -_ = s2:create_index('pk') _ = s2:insert{555, 'eee'} s1:select() -- 2.11.0