Tarantool development patches archive
 help / color / mirror / Atom feed
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

  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