* [PATCH 1/2] memtx: add yields during space format check
2019-06-03 15:51 [PATCH 0/2] memtx: make space format check yield occasionally Serge Petrenko
@ 2019-06-03 15:51 ` Serge Petrenko
2019-06-03 15:54 ` [tarantool-patches] " Serge Petrenko
2019-06-04 15:17 ` Vladimir Davydov
2019-06-03 15:51 ` [PATCH 2/2] test: move vinyl space format test case to engine suite Serge Petrenko
1 sibling, 2 replies; 5+ messages in thread
From: Serge Petrenko @ 2019-06-03 15:51 UTC (permalink / raw)
To: vdavydov.dev; +Cc: tarantool-patches, Serge Petrenko
Just like index build, space check in memtx stalls event loop for the
whole check time. Add occasional yields, and an on_replace trigger,
which checks format for tuples inserted while space format check is in
progress.
Follow-up #3976
@TarantoolBol document
Title: memtx now checks space format in background
There is no event loop stall when memtx engine checks space format
anymore. You may insert tuples into a space, while its new format is
being checked. If the tuples don't match the new format, format change
will be aborted.
---
src/box/memtx_space.c | 110 ++++++++++++++++++++++++++++++++----------
1 file changed, 84 insertions(+), 26 deletions(-)
diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index 1d209033c..a370c038a 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -43,6 +43,16 @@
#include "column_mask.h"
#include "sequence.h"
+/*
+ * Yield every 1K tuples.
+ * In debug mode yield more often for testing purposes.
+ */
+#ifdef NDEBUG
+enum { MEMTX_YIELD_LOOPS = 1000 };
+#else
+enum { MEMTX_YIELD_LOOPS = 10 };
+#endif
+
static void
memtx_space_destroy(struct space *space)
{
@@ -814,6 +824,52 @@ memtx_space_add_primary_key(struct space *space)
return 0;
}
+/*
+ * Ongoing index build or format check state used by
+ * corrseponding on_replace triggers.
+ */
+struct memtx_op_state {
+ /* The index being built. */
+ struct index *index;
+ /* New format to be enforced. */
+ struct tuple_format *format;
+ /* Operation cursor. Marks the last processed tuple to date */
+ struct tuple *cursor;
+ /* Primary key key_def to compare new tuples with cursor. */
+ struct key_def *cmp_def;
+ struct diag diag;
+ int rc;
+};
+
+static void
+memtx_check_on_replace(struct trigger *trigger, void *event)
+{
+ struct txn *txn = event;
+ struct memtx_op_state *state = trigger->data;
+ struct txn_stmt *stmt = txn_current_stmt(txn);
+
+ /* Nothing to check on DELETE. */
+ if (stmt->new_tuple == NULL)
+ return;
+
+ /* We have already failed. */
+ if (state->rc != 0)
+ return;
+
+ /*
+ * Only check format for already processed part of the space,
+ * all the tuples inserted below cursor will be checked by the
+ * main routine later.
+ */
+ if (tuple_compare(state->cursor, HINT_NONE, stmt->new_tuple, HINT_NONE,
+ state->cmp_def) < 0)
+ return;
+
+ state->rc = tuple_validate(state->format, stmt->new_tuple);
+ if (state->rc != 0)
+ diag_move(diag_get(), &state->diag);
+}
+
static int
memtx_space_check_format(struct space *space, struct tuple_format *format)
{
@@ -827,8 +883,19 @@ memtx_space_check_format(struct space *space, struct tuple_format *format)
if (it == NULL)
return -1;
+ struct memtx_op_state state;
+ state.format = format;
+ state.cmp_def = pk->def->key_def;
+ state.rc = 0;
+ diag_create(&state.diag);
+
+ struct trigger on_replace;
+ trigger_create(&on_replace, memtx_check_on_replace, &state, NULL);
+ trigger_add(&space->on_replace, &on_replace);
+
int rc;
struct tuple *tuple;
+ size_t count = 0;
while ((rc = iterator_next(it, &tuple)) == 0 && tuple != NULL) {
/*
* Check that the tuple is OK according to the
@@ -837,8 +904,22 @@ memtx_space_check_format(struct space *space, struct tuple_format *format)
rc = tuple_validate(format, tuple);
if (rc != 0)
break;
+ if (++count % MEMTX_YIELD_LOOPS == 0) {
+ state.cursor = tuple;
+ tuple_ref(state.cursor);
+ fiber_sleep(0);
+ tuple_unref(state.cursor);
+
+ if (state.rc != 0) {
+ rc = -1;
+ diag_move(&state.diag, diag_get());
+ break;
+ }
+ }
}
iterator_delete(it);
+ diag_destroy(&state.diag);
+ trigger_clear(&on_replace);
return rc;
}
@@ -870,25 +951,11 @@ memtx_init_ephemeral_space(struct space *space)
memtx_space_add_primary_key(space);
}
-/* Ongoing index build state used by memtx_build_on_replace triggers. */
-struct memtx_build_state {
- /* The index being built. */
- struct index *index;
- /* New index format to be enforced. */
- struct tuple_format *format;
- /* Index build cursor. Marks the last tuple inserted to date */
- struct tuple *cursor;
- /* Primary key key_def to compare inserted tuples with cursor. */
- struct key_def *cmp_def;
- struct diag diag;
- int rc;
-};
-
static void
memtx_build_on_replace(struct trigger *trigger, void *event)
{
struct txn *txn = event;
- struct memtx_build_state *state = trigger->data;
+ struct memtx_op_state *state = trigger->data;
struct txn_stmt *stmt = txn_current_stmt(txn);
struct tuple *cmp_tuple = stmt->new_tuple != NULL ? stmt->new_tuple :
@@ -925,15 +992,6 @@ static int
memtx_space_build_index(struct space *src_space, struct index *new_index,
struct tuple_format *new_format)
{
- /*
- * Yield every 1K tuples.
- * In debug mode yield more often for testing purposes.
- */
-#ifdef NDEBUG
- enum { YIELD_LOOPS = 1000 };
-#else
- enum { YIELD_LOOPS = 10 };
-#endif
/**
* If it's a secondary key, and we're not building them
* yet (i.e. it's snapshot recovery for memtx), do nothing.
@@ -959,7 +1017,7 @@ memtx_space_build_index(struct space *src_space, struct index *new_index,
if (it == NULL)
return -1;
- struct memtx_build_state state;
+ struct memtx_op_state state;
state.index = new_index;
state.format = new_format;
state.cmp_def = pk->def->key_def;
@@ -1005,7 +1063,7 @@ memtx_space_build_index(struct space *src_space, struct index *new_index,
*/
if (new_index->def->iid == 0)
tuple_ref(tuple);
- if (++count % YIELD_LOOPS == 0) {
+ if (++count % MEMTX_YIELD_LOOPS == 0) {
/*
* Remember the latest inserted tuple to
* avoid processing yet to be added tuples
--
2.20.1 (Apple Git-117)
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 2/2] test: move vinyl space format test case to engine suite
2019-06-03 15:51 [PATCH 0/2] memtx: make space format check yield occasionally Serge Petrenko
2019-06-03 15:51 ` [PATCH 1/2] memtx: add yields during space format check Serge Petrenko
@ 2019-06-03 15:51 ` Serge Petrenko
1 sibling, 0 replies; 5+ messages in thread
From: Serge Petrenko @ 2019-06-03 15:51 UTC (permalink / raw)
To: vdavydov.dev; +Cc: tarantool-patches, Serge Petrenko
After making memtx space format check non-blocking, move the appropriate
vinyl test case to engine suite. Introduce a new errinj,
ERRINJ_CHECK_FORMAT_DELAY, to unify the test case for both engines.
Follow-up #3976
---
src/box/memtx_space.c | 15 +++++++
src/box/vinyl.c | 6 +++
src/lib/core/errinj.h | 1 +
test/box/errinj.result | 36 ++++++++--------
test/engine/errinj_ddl.result | 68 ++++++++++++++++++++++++++++++
test/engine/errinj_ddl.test.lua | 33 +++++++++++++++
test/vinyl/errinj_ddl.result | 75 ---------------------------------
test/vinyl/errinj_ddl.test.lua | 35 ---------------
8 files changed, 142 insertions(+), 127 deletions(-)
diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index a370c038a..54d08b531 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -916,6 +916,21 @@ memtx_space_check_format(struct space *space, struct tuple_format *format)
break;
}
}
+
+ struct errinj *inj = errinj(ERRINJ_CHECK_FORMAT_DELAY, ERRINJ_BOOL);
+ if (inj != NULL && inj->bparam && count == 1) {
+ state.cursor = tuple;
+ tuple_ref(state.cursor);
+ do {
+ fiber_sleep(0);
+ } while (inj->bparam);
+ tuple_unref(state.cursor);
+ if (state.rc != 0) {
+ rc = -1;
+ diag_move(&state.diag, diag_get());
+ break;
+ }
+ }
}
iterator_delete(it);
diag_destroy(&state.diag);
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 9b4bc53bf..0c80496d9 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -1156,6 +1156,12 @@ vinyl_space_check_format(struct space *space, struct tuple_format *format)
*/
if (++loops % VY_YIELD_LOOPS == 0)
fiber_sleep(0);
+ struct errinj *inj = errinj(ERRINJ_CHECK_FORMAT_DELAY, ERRINJ_BOOL);
+ if (inj != NULL && inj->bparam && loops == 1) {
+ do {
+ fiber_sleep(0);
+ } while (inj->bparam);
+ }
if (ctx.is_failed) {
diag_move(&ctx.diag, diag_get());
rc = -1;
diff --git a/src/lib/core/errinj.h b/src/lib/core/errinj.h
index 796d3a194..787836514 100644
--- a/src/lib/core/errinj.h
+++ b/src/lib/core/errinj.h
@@ -107,6 +107,7 @@ struct errinj {
_(ERRINJ_XLOG_READ, ERRINJ_INT, {.iparam = -1}) \
_(ERRINJ_VYRUN_INDEX_GARBAGE, ERRINJ_BOOL, {.bparam = false}) \
_(ERRINJ_VYRUN_DATA_READ, ERRINJ_BOOL, {.bparam = false}) \
+ _(ERRINJ_CHECK_FORMAT_DELAY, ERRINJ_BOOL, {.bparam = false}) \
_(ERRINJ_BUILD_INDEX, ERRINJ_INT, {.iparam = -1}) \
_(ERRINJ_BUILD_INDEX_DELAY, ERRINJ_BOOL, {.bparam = false}) \
_(ERRINJ_VY_POINT_ITER_WAIT, ERRINJ_BOOL, {.bparam = false}) \
diff --git a/test/box/errinj.result b/test/box/errinj.result
index 5759e7494..f24bf7813 100644
--- a/test/box/errinj.result
+++ b/test/box/errinj.result
@@ -36,32 +36,34 @@ errinj.info()
state: -1
ERRINJ_WAL_WRITE_EOF:
state: false
+ ERRINJ_COIO_SENDFILE_CHUNK:
+ state: -1
ERRINJ_VYRUN_INDEX_GARBAGE:
state: false
ERRINJ_BUILD_INDEX_DELAY:
state: false
- ERRINJ_COIO_SENDFILE_CHUNK:
- state: -1
- ERRINJ_VY_INDEX_FILE_RENAME:
- state: false
- ERRINJ_VY_POINT_ITER_WAIT:
+ ERRINJ_VY_RUN_FILE_RENAME:
state: false
+ ERRINJ_BUILD_INDEX:
+ state: -1
+ ERRINJ_RELAY_BREAK_LSN:
+ state: -1
ERRINJ_VY_DELAY_PK_LOOKUP:
state: false
ERRINJ_VY_TASK_COMPLETE:
state: false
ERRINJ_PORT_DUMP:
state: false
- ERRINJ_WAL_BREAK_LSN:
- state: -1
+ ERRINJ_CHECK_FORMAT_DELAY:
+ state: false
ERRINJ_WAL_IO:
state: false
ERRINJ_WAL_FALLOCATE:
state: 0
- ERRINJ_LOG_ROTATE:
- state: false
ERRINJ_VY_DUMP_DELAY:
state: false
+ ERRINJ_WAL_BREAK_LSN:
+ state: -1
ERRINJ_TUPLE_FORMAT_COUNT:
state: -1
ERRINJ_TUPLE_ALLOC:
@@ -72,25 +74,25 @@ errinj.info()
state: false
ERRINJ_RELAY_REPORT_INTERVAL:
state: 0
- ERRINJ_RELAY_BREAK_LSN:
- state: -1
+ ERRINJ_VY_INDEX_FILE_RENAME:
+ state: false
ERRINJ_VY_READ_PAGE_TIMEOUT:
state: 0
ERRINJ_XLOG_META:
state: false
ERRINJ_SIO_READ_MAX:
state: -1
- ERRINJ_VY_RUN_FILE_RENAME:
+ ERRINJ_VY_LOG_FILE_RENAME:
state: false
ERRINJ_WAL_WRITE_DISK:
state: false
- ERRINJ_VY_LOG_FILE_RENAME:
- state: false
ERRINJ_HTTP_RESPONSE_ADD_WAIT:
state: false
+ ERRINJ_SNAP_COMMIT_DELAY:
+ state: false
ERRINJ_VY_RUN_WRITE:
state: false
- ERRINJ_SNAP_COMMIT_DELAY:
+ ERRINJ_LOG_ROTATE:
state: false
ERRINJ_VY_LOG_FLUSH_DELAY:
state: false
@@ -104,10 +106,10 @@ errinj.info()
state: false
ERRINJ_WAL_ROTATE:
state: false
- ERRINJ_BUILD_INDEX:
- state: -1
ERRINJ_RELAY_EXIT_DELAY:
state: 0
+ ERRINJ_VY_POINT_ITER_WAIT:
+ state: false
ERRINJ_MEMTX_DELAY_GC:
state: false
ERRINJ_IPROTO_TX_DELAY:
diff --git a/test/engine/errinj_ddl.result b/test/engine/errinj_ddl.result
index 7396de7c6..5f4043005 100644
--- a/test/engine/errinj_ddl.result
+++ b/test/engine/errinj_ddl.result
@@ -10,6 +10,74 @@ engine = test_run:get_cfg('engine')
errinj = box.error.injection
---
...
+--
+-- Check that ALTER is abroted if a tuple inserted during space
+-- format change does not conform to the new format.
+--
+format = {}
+---
+...
+format[1] = {name = 'field1', type = 'unsigned'}
+---
+...
+format[2] = {name = 'field2', type = 'string', is_nullable = true}
+---
+...
+s = box.schema.space.create('test', {engine = engine, format = format})
+---
+...
+_ = s:create_index('pk')
+---
+...
+pad = string.rep('x', 16)
+---
+...
+for i = 101, 200 do s:replace{i, pad} end
+---
+...
+ch = fiber.channel(1)
+---
+...
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+_ = fiber.create(function()
+ fiber.sleep(0.01)
+ for i = 1, 100 do
+ s:replace{i, box.NULL}
+ end
+ errinj.set("ERRINJ_CHECK_FORMAT_DELAY", false)
+ ch:put(true)
+end);
+---
+...
+test_run:cmd("setopt delimiter ''");
+---
+- true
+...
+format[2].is_nullable = false
+---
+...
+errinj.set("ERRINJ_CHECK_FORMAT_DELAY", true)
+---
+- ok
+...
+s:format(format) -- must fail
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected string'
+...
+ch:get()
+---
+- true
+...
+s:count() -- 200
+---
+- 200
+...
+s:drop()
+---
+...
t = box.schema.space.create("space", {engine = engine})
---
...
diff --git a/test/engine/errinj_ddl.test.lua b/test/engine/errinj_ddl.test.lua
index f67ca5055..a382daa44 100644
--- a/test/engine/errinj_ddl.test.lua
+++ b/test/engine/errinj_ddl.test.lua
@@ -3,6 +3,39 @@ fiber = require('fiber')
engine = test_run:get_cfg('engine')
errinj = box.error.injection
+--
+-- Check that ALTER is abroted if a tuple inserted during space
+-- format change does not conform to the new format.
+--
+format = {}
+format[1] = {name = 'field1', type = 'unsigned'}
+format[2] = {name = 'field2', type = 'string', is_nullable = true}
+s = box.schema.space.create('test', {engine = engine, format = format})
+_ = s:create_index('pk')
+
+pad = string.rep('x', 16)
+for i = 101, 200 do s:replace{i, pad} end
+
+ch = fiber.channel(1)
+test_run:cmd("setopt delimiter ';'")
+_ = fiber.create(function()
+ fiber.sleep(0.01)
+ for i = 1, 100 do
+ s:replace{i, box.NULL}
+ end
+ errinj.set("ERRINJ_CHECK_FORMAT_DELAY", false)
+ ch:put(true)
+end);
+test_run:cmd("setopt delimiter ''");
+
+format[2].is_nullable = false
+errinj.set("ERRINJ_CHECK_FORMAT_DELAY", true)
+s:format(format) -- must fail
+ch:get()
+
+s:count() -- 200
+s:drop()
+
t = box.schema.space.create("space", {engine = engine})
_ = t:create_index("pk")
for i = 1,1000 do t:insert{i, i} end
diff --git a/test/vinyl/errinj_ddl.result b/test/vinyl/errinj_ddl.result
index 4c3df3acd..8fe9064b8 100644
--- a/test/vinyl/errinj_ddl.result
+++ b/test/vinyl/errinj_ddl.result
@@ -8,81 +8,6 @@ errinj = box.error.injection
---
...
--
--- Check that ALTER is abroted if a tuple inserted during space
--- format change does not conform to the new format.
---
-format = {}
----
-...
-format[1] = {name = 'field1', type = 'unsigned'}
----
-...
-format[2] = {name = 'field2', type = 'string', is_nullable = true}
----
-...
-s = box.schema.space.create('test', {engine = 'vinyl', format = format})
----
-...
-_ = s:create_index('pk', {page_size = 16})
----
-...
-pad = string.rep('x', 16)
----
-...
-for i = 101, 200 do s:replace{i, pad} end
----
-...
-box.snapshot()
----
-- ok
-...
-ch = fiber.channel(1)
----
-...
-test_run:cmd("setopt delimiter ';'")
----
-- true
-...
-_ = fiber.create(function()
- fiber.sleep(0.01)
- for i = 1, 100 do
- s:replace{i, box.NULL}
- end
- ch:put(true)
-end);
----
-...
-test_run:cmd("setopt delimiter ''");
----
-- true
-...
-errinj.set("ERRINJ_VY_READ_PAGE_TIMEOUT", 0.001)
----
-- ok
-...
-format[2].is_nullable = false
----
-...
-s:format(format) -- must fail
----
-- error: 'Tuple field 2 type does not match one required by operation: expected string'
-...
-errinj.set("ERRINJ_VY_READ_PAGE_TIMEOUT", 0)
----
-- ok
-...
-ch:get()
----
-- true
-...
-s:count() -- 200
----
-- 200
-...
-s:drop()
----
-...
---
-- Check that modifications done to the space during the final dump
-- of a newly built index are recovered properly.
--
diff --git a/test/vinyl/errinj_ddl.test.lua b/test/vinyl/errinj_ddl.test.lua
index 830d1d1be..3bc8bed80 100644
--- a/test/vinyl/errinj_ddl.test.lua
+++ b/test/vinyl/errinj_ddl.test.lua
@@ -2,41 +2,6 @@ test_run = require('test_run').new()
fiber = require('fiber')
errinj = box.error.injection
---
--- Check that ALTER is abroted if a tuple inserted during space
--- format change does not conform to the new format.
---
-format = {}
-format[1] = {name = 'field1', type = 'unsigned'}
-format[2] = {name = 'field2', type = 'string', is_nullable = true}
-s = box.schema.space.create('test', {engine = 'vinyl', format = format})
-_ = s:create_index('pk', {page_size = 16})
-
-pad = string.rep('x', 16)
-for i = 101, 200 do s:replace{i, pad} end
-box.snapshot()
-
-ch = fiber.channel(1)
-test_run:cmd("setopt delimiter ';'")
-_ = fiber.create(function()
- fiber.sleep(0.01)
- for i = 1, 100 do
- s:replace{i, box.NULL}
- end
- ch:put(true)
-end);
-test_run:cmd("setopt delimiter ''");
-
-errinj.set("ERRINJ_VY_READ_PAGE_TIMEOUT", 0.001)
-format[2].is_nullable = false
-s:format(format) -- must fail
-errinj.set("ERRINJ_VY_READ_PAGE_TIMEOUT", 0)
-
-ch:get()
-
-s:count() -- 200
-s:drop()
-
--
-- Check that modifications done to the space during the final dump
-- of a newly built index are recovered properly.
--
2.20.1 (Apple Git-117)
^ permalink raw reply [flat|nested] 5+ messages in thread