Re: [Tarantool-patches] [PATCH 1/2] errinj: introduce delayed injection

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch!

> diff --git a/src/errinj.h b/src/errinj.h
> index 2cb090b68..990f4921d 100644
> --- a/src/errinj.h
> +++ b/src/errinj.h
> @@ -149,6 +149,7 @@ errinj_foreach(errinj_cb cb, void *cb_ctx);
>  #ifdef NDEBUG
>  #  define ERROR_INJECT(ID, CODE)
>  #  define errinj(ID, TYPE) ((struct errinj *) NULL)
> +#  define ERROR_INJECT_DELAYED(ID, DELAY, CODE)
>  #else
>  #  /* Returns the error injection by id */
>  #  define errinj(ID, TYPE) \
> @@ -162,6 +163,14 @@ errinj_foreach(errinj_cb cb, void *cb_ctx);
>  		if (errinj(ID, ERRINJ_BOOL)->bparam) \
>  			CODE; \
>  	} while (0)
> +#  define ERROR_INJECT_DELAYED(ID, DELAY, CODE) \
> +	do { \
> +		static int _DELAY##ID = DELAY; \
> +		if (errinj(ID, ERRINJ_BOOL)->bparam) { \
> +			if (_DELAY##ID-- == 0) \
> +				CODE; \
> +		} \
> +	} while (0)

The method is fine unless you will ever want to set the same
error injection twice without restarting Tarantool. With this
solution there is no way to reset _DELAY##ID back to the same or
a different value. It will stay 0 until restart. This a serious
enough problem to reconsider the approach. There are injections
used in multiple tests, and we can't count on restart after each
one.

This is the reason why you won't be able to use this delayed
injection for VY_STMT_ALLOC in 'uninitialized memory' patchset.

'Delayed' easily can be implemented by iparam. For example, look
at ERRINJ_WAL_FALLOCATE and your own VY_STMT_ALLOC. You set
iparam to some value, then you decrement it until 0, and inject
the error at this moment. Works perfectly fine, and without
introducing any new unrecoverable ERRINJ_ counters. However if you
want to simplify that, just wrap the iparam-method into a macros.
Even the same name could be fine - ERROR_INJECT_DELAYED.

Moreover, it simplifies check that the injection really happened,
and its progress. You just look at iparam in Lua and see how many
times it has executed, and whether it is equal to 0 already
(with bparam you can do the same though, by setting it to false
explicitly).