From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp53.i.mail.ru (smtp53.i.mail.ru [94.100.177.113]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 2BCF5469710 for ; Thu, 7 May 2020 16:50:02 +0300 (MSK) Date: Thu, 7 May 2020 13:50:01 +0000 From: Nikita Pettik Message-ID: <20200507135001.GC11724@tarantool.org> References: <1fb821b72a258f638109ea5e2e9a6bfd6106b9da.1588273848.git.korablev@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH 1/2] errinj: introduce delayed injection List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org On 03 May 21:20, Vladislav Shpilevoy wrote: > 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. Yep, this is obvious drawback of chosen implementation. > 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. Ok, here's diff: diff --git a/src/errinj.h b/src/errinj.h index 990f4921d..945abb939 100644 --- a/src/errinj.h +++ b/src/errinj.h @@ -163,12 +163,10 @@ errinj_foreach(errinj_cb cb, void *cb_ctx); if (errinj(ID, ERRINJ_BOOL)->bparam) \ CODE; \ } while (0) -# define ERROR_INJECT_DELAYED(ID, DELAY, CODE) \ +# define ERROR_INJECT_COUNTDOWN(ID, CODE) \ do { \ - static int _DELAY##ID = DELAY; \ - if (errinj(ID, ERRINJ_BOOL)->bparam) { \ - if (_DELAY##ID-- == 0) \ - CODE; \ + if (errinj(ID, ERRINJ_INT)->iparam-- == 0) { \ + CODE; \ } \ } while (0) #endif > 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).