[PATCH] recovery: remove yields from index build and format check
Vladimir Davydov
vdavydov.dev at gmail.com
Thu Jun 13 19:21:48 MSK 2019
On Thu, Jun 13, 2019 at 06:30:38PM +0300, Serge Petrenko wrote:
> Iproto already listens for requests during recovery, so yielding at this
> point of time allows such early requests, which arrived during recovery,
> be processed while data is in unfinished state. This caused box/net.box
> test failures, and is potentially harmful.
> Besides, there is no need to yield during recovery.
>
> Closes #4273
> ---
> https://github.com/tarantool/tarantool/issues/4273
> https://github.com/tarantool/tarantool/tree/sp/gh-4273-dont-yield-on-recovery
>
> src/box/memtx_space.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
Pushed to master with a few nitpicks (see below).
>
> diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
> index 9f232803e..3e607e0e6 100644
> --- a/src/box/memtx_space.c
> +++ b/src/box/memtx_space.c
> @@ -47,6 +47,9 @@
> * Yield every 1K tuples while building a new index or checking
> * a space format. In debug mode yield more often for testing
> * purposes.
> + * Yields do not happen during recovery. At this point of time
> + * iproto aready accepts requests, and yielding would allow them
> + * to be proccessed while data is not fully recovered.
Added a newline between paragraphs - text looks more readable that way.
> */
> #ifdef NDEBUG
> enum { MEMTX_DDL_YIELD_LOOPS = 1000 };
> @@ -884,6 +887,7 @@ memtx_space_check_format(struct space *space, struct tuple_format *format)
> if (it == NULL)
> return -1;
>
> + struct memtx_engine *memtx = (struct memtx_engine *)space->engine;
> struct memtx_ddl_state state;
> state.format = format;
> state.cmp_def = pk->def->key_def;
> @@ -905,7 +909,7 @@ memtx_space_check_format(struct space *space, struct tuple_format *format)
> rc = tuple_validate(format, tuple);
> if (rc != 0)
> break;
> - if (++count % MEMTX_DDL_YIELD_LOOPS == 0) {
> + if (++count % MEMTX_DDL_YIELD_LOOPS == 0 && memtx->state == MEMTX_OK) {
The line's too long. Split.
> state.cursor = tuple;
> tuple_ref(state.cursor);
> fiber_sleep(0);
> @@ -1033,6 +1037,7 @@ memtx_space_build_index(struct space *src_space, struct index *new_index,
> if (it == NULL)
> return -1;
>
> + struct memtx_engine *memtx = (struct memtx_engine *)src_space->engine;
> struct memtx_ddl_state state;
> state.index = new_index;
> state.format = new_format;
> @@ -1079,7 +1084,7 @@ memtx_space_build_index(struct space *src_space, struct index *new_index,
> */
> if (new_index->def->iid == 0)
> tuple_ref(tuple);
> - if (++count % MEMTX_DDL_YIELD_LOOPS == 0) {
> + if (++count % MEMTX_DDL_YIELD_LOOPS == 0 && memtx->state == MEMTX_OK) {
Ditto.
More information about the Tarantool-patches
mailing list