[PATCH 5/6] vinyl: don't yield while logging index creation

Vladimir Davydov vdavydov.dev at gmail.com
Sun Jun 30 22:40:18 MSK 2019


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




More information about the Tarantool-patches mailing list