Re: [PATCH] recovery: remove yields from index build and format check

[Vladimir Davydov <vdavydov.dev@gmail.com>]

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.