From: Vladimir Davydov <vdavydov.dev@gmail.com> To: kostja@tarantool.org Cc: tarantool-patches@freelists.org Subject: [PATCH 5/6] vinyl: don't yield while logging index creation Date: Sun, 30 Jun 2019 22:40:18 +0300 [thread overview] Message-ID: <125bf3791e1d9fdf00720d0b7e70f818c05000c8.1561922496.git.vdavydov.dev@gmail.com> (raw) In-Reply-To: <cover.1561922496.git.vdavydov.dev@gmail.com> In-Reply-To: <cover.1561922496.git.vdavydov.dev@gmail.com> 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
next prev parent reply other threads:[~2019-06-30 19:40 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-30 19:40 [PATCH 0/6] Get rid of the schema lock Vladimir Davydov 2019-06-30 19:40 ` [PATCH 1/6] Add ERROR_INJECT_YIELD and ERROR_INJECT_SLEEP helpers Vladimir Davydov 2019-07-03 19:12 ` Konstantin Osipov 2019-07-04 15:50 ` Vladimir Davydov 2019-06-30 19:40 ` [PATCH 2/6] Replace ERRINJ_SNAP_WRITE_ROW_TIMEOUT with ERRINJ_SNAP_WRITE_DELAY Vladimir Davydov 2019-07-03 19:13 ` Konstantin Osipov 2019-07-04 15:51 ` Vladimir Davydov 2019-06-30 19:40 ` [PATCH 3/6] Don't take schema lock for checkpointing Vladimir Davydov 2019-07-03 19:21 ` Konstantin Osipov 2019-07-03 20:05 ` Vladimir Davydov 2019-06-30 19:40 ` [PATCH 4/6] test: make vinyl/replica_rejoin more stable Vladimir Davydov 2019-07-03 19:23 ` Konstantin Osipov 2019-07-04 15:51 ` Vladimir Davydov 2019-06-30 19:40 ` Vladimir Davydov [this message] 2019-06-30 19:40 ` [PATCH 6/6] Replace schema lock with fine-grained locking Vladimir Davydov 2019-07-03 19:35 ` Konstantin Osipov 2019-07-03 19:56 ` Vladimir Davydov 2019-07-04 8:09 ` Konstantin Osipov 2019-07-04 17:06 ` Vladimir Davydov 2019-07-08 7:40 ` Konstantin Osipov 2019-07-08 8:41 ` Vladimir Davydov 2019-07-05 8:53 ` [PATCH 0/6] Get rid of the schema lock Vladimir Davydov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=125bf3791e1d9fdf00720d0b7e70f818c05000c8.1561922496.git.vdavydov.dev@gmail.com \ --to=vdavydov.dev@gmail.com \ --cc=kostja@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH 5/6] vinyl: don'\''t yield while logging index creation' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox