From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 4276F6EC59; Wed, 10 Mar 2021 15:48:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 4276F6EC59 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1615380538; bh=YV0RiV9V8uGZCjO/RhTqUUlgt0x/pLIib9kMW5pltcg=; h=To:References:Date:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=IDnWKS97SYRXJ6fE7B7KNicXF4WWZZDiqhNGmzCVI/RRBR5Pn8uLQEJnPK2YVAzze S4i4yYFTRqI3tExUMtR+ACzBrQtzb69JHUGRDb3jFJFQ4tcs5ukRfBzn4R7wHt3PfG yZVSrITYjt3RlA619JUyneiNKGseGfpKTQeR+Gv0= Received: from smtp47.i.mail.ru (smtp47.i.mail.ru [94.100.177.107]) (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 CB7DF6EC59 for ; Wed, 10 Mar 2021 15:48:56 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org CB7DF6EC59 Received: by smtp47.i.mail.ru with esmtpa (envelope-from ) id 1lJyGl-0000zZ-AG; Wed, 10 Mar 2021 15:48:56 +0300 To: Leonid Vasiliev , Alexander Turenko References: Message-ID: Date: Wed, 10 Mar 2021 15:48:54 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: ru X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9D3134714A9BDB69B7D7089AC2F541D13D72A77FF59123A9B00894C459B0CD1B9CB55541A9BCFBE1995FE0D0DEAC9B81F619C2FB7EFA4D11C5872F0C4753F95D2 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE72E2D36A15E1833D8EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063748E7A03516F25E8E8638F802B75D45FF5571747095F342E8C7A0BC55FA0FE5FC2DC3353C0112CD4BEACE7DBA5785CE51A0F60042BFDBA846389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C0D9442B0B5983000E8941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B64854413538E1713FCC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB8D32BA5DBAC0009BE9E8FC8737B5C22497984FED25317617E76E601842F6C81A12EF20D2F80756B5F7E9C4E3C761E06A776E601842F6C81A127C277FBC8AE2E8B4DD03FB0832A26533AA81AA40904B5D9DBF02ECDB25306B2B25CBF701D1BE8734AD6D5ED66289B5278DA827A17800CE78DD9044B304389D467F23339F89546C5A8DF7F3B2552694A6FED454B719173D6725E5C173C3A84C3D959F89D3055010035872C767BF85DA2F004C906525384306FED454B719173D6462275124DF8B9C9DE2850DD75B2526BE5BFE6E7EFDEDCD789D4C264860C145E X-C1DE0DAB: 0D63561A33F958A5BE5A4683C5E8C721EFF390B5590743DD5E82A365E0504138D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75448CF9D3A7B2C848410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D341E08D7EE1804B763E746CD4DBB20A4F6A08F2BC0F6E247048CFDD088C66DC9A7C52D61A5DB7F29311D7E09C32AA3244C7012F530F4B2D78DA2A4531A371A74EA30363D8B7DA7DD44927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojsR8tyFmO15Nwpu8ef7k1FA== X-Mailru-Sender: 49D287FBCBBF3A5C86449ED72883D43A279205B38A2FF51764BE4A9A7C12CF295AB84C59671071102325CE592137BF811458020726E2BC9F60B790B42577D741D24DB76E211E293F7402F9BA4338D657ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCHv3 1/2] core: add setting error injections via env X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Artem via Tarantool-patches Reply-To: Artem Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, thank you for the review! Created wiki for this new ability, see https://github.com/tarantool/tarantool/wiki/Error-injections . Each your review comment is done (see below) and changes you can see on a branch. 05.03.2021 15:11, Leonid Vasiliev пишет: > Hi! Thank you for the patch. > > On 3/4/21 12:15 PM, Artem Starshov wrote: >> Sometimes, it's useful to set error injections via environment >> variables and this commit adds this opportunity. >> >> e.g: `ERRINJ_WAL_WRITE=true tarantool` will be launched with >> ERRINJ_WAL_WRITE setted to true. >> >> Errinjs with bool parameters can be set to "true", "false", >> "True", "False", "TRUE", "FALSE", etc. (case-insensitive variable). >> >> Errinjs with int or double parameters should be whole valid ("123s" >> is invalid). >> e.g. for int or double: "123", "-1", "2.34", "+2.34". >> >> NOTE: errinjs should work only in debug mode, so added >> `release_disabled` in >> suite.ini. But there is a bug in test-run: `release_disable` disables >> tests at >> each build type. Partially this problem is descripted in >> tarantool/test-run#199. >> >> Part of #5040 >> --- >>   src/lib/core/errinj.c                         | 30 ++++++++++++++++ >>   src/lib/core/errinj.h                         |  5 +++ >>   src/main.cc                                   |  3 ++ >>   .../errinj_set_with_enviroment_vars.test.lua  | 29 ++++++++++++++++ >>   ...errinj_set_with_enviroment_vars_script.lua | 34 +++++++++++++++++++ >>   test/box-tap/suite.ini                        |  1 + >>   6 files changed, 102 insertions(+) >>   create mode 100755 >> test/box-tap/errinj_set_with_enviroment_vars.test.lua >>   create mode 100644 >> test/box-tap/errinj_set_with_enviroment_vars_script.lua >> >> diff --git a/src/lib/core/errinj.c b/src/lib/core/errinj.c >> index d3aa0ca4f..8f76bfac1 100644 >> --- a/src/lib/core/errinj.c >> +++ b/src/lib/core/errinj.c >> @@ -28,9 +28,11 @@ >>    * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF >>    * SUCH DAMAGE. >>    */ >> +#include >>   #include >>   #include >>   #include >> +#include >>   #include >>     #include "trivia/config.h" >> @@ -66,3 +68,31 @@ int errinj_foreach(errinj_cb cb, void *cb_ctx) { >>       } >>       return 0; >>   } >> + >> +void errinj_set_with_environment_vars(void) { >> +    for (enum errinj_id i = 0; i < errinj_id_MAX; i++) { >> +        struct errinj *inj = &errinjs[i]; >> +        const char *env_value = getenv(inj->name); >> +        if (env_value == NULL || *env_value == '\0') >> +            continue; >> + >> +        if (inj->type == ERRINJ_INT) { >> +            char *end; >> +            int64_t int_value = strtoll(env_value, &end, 10); >> +            if (*end == '\0') > > "Seems like the `panic()` should be called in the case of `else()`. Here > and in other cases of parsing." is about this `if`. > Done. >> +                inj->iparam = int_value; >> +        } else if (inj->type == ERRINJ_BOOL) { >> +            if (strcasecmp(env_value, "false") == 0) > > > "Seems like the `panic()` should be called in the case of `else()`. Here > and in other cases of parsing." is about this `if`. > Done. >> +                inj->bparam = false; >> +            else if (strcasecmp(env_value, "true") == 0) >> +                inj->bparam = true; >> +        } else if (inj->type == ERRINJ_DOUBLE) { >> +            char *end; >> +            double double_value = strtod(env_value, &end); >> +            if (*end == '\0') > > > "Seems like the `panic()` should be called in the case of `else()`. Here > and in other cases of parsing." is about this `if`. > Done. >> +                inj->dparam = double_value; >> +        } else { > > This `else` is unnecessary, because these structures are from a > hardcoded list. > Done. Removed. >> +            panic("Unknown errinj type received"); >> +        } >> +    } >> +} >> diff --git a/src/lib/core/errinj.h b/src/lib/core/errinj.h >> index 814c57c2e..10edbe2b9 100644 >> --- a/src/lib/core/errinj.h >> +++ b/src/lib/core/errinj.h >> @@ -168,6 +168,11 @@ typedef int (*errinj_cb)(struct errinj *e, void >> *cb_ctx); >>   int >>   errinj_foreach(errinj_cb cb, void *cb_ctx); >>   +/** >> + * Set injections by scanning ERRINJ_$(NAME) in environment variables >> + */ >> +void errinj_set_with_environment_vars(void); >> + >>   #ifdef NDEBUG >>   #  define ERROR_INJECT(ID, CODE) >>   #  define ERROR_INJECT_WHILE(ID, CODE) >> diff --git a/src/main.cc b/src/main.cc >> index 2fce81bb3..718180b80 100644 >> --- a/src/main.cc >> +++ b/src/main.cc >> @@ -710,6 +710,9 @@ main(int argc, char **argv) >>       memtx_tx_manager_init(); >>       crypto_init(); >>       systemd_init(); >> +#ifndef NDEBUG >> +    errinj_set_with_environment_vars(); >> +#endif >>       tarantool_lua_init(tarantool_bin, main_argc, main_argv); >>         start_time = ev_monotonic_time(); >> diff --git a/test/box-tap/errinj_set_with_enviroment_vars.test.lua >> b/test/box-tap/errinj_set_with_enviroment_vars.test.lua >> new file mode 100755 >> index 000000000..8d4ecaedd >> --- /dev/null >> +++ b/test/box-tap/errinj_set_with_enviroment_vars.test.lua >> @@ -0,0 +1,29 @@ >> +#!/usr/bin/env tarantool >> +local fio = require('fio') >> + >> +-- Execute errinj_set_with_enviroment_vars_script.lua >> +-- via tarantool with presetted environment variables. >> +local TARANTOOL_PATH = arg[-1] >> +local bool_env     =    'ERRINJ_TESTING=true ' .. > > I don't mind one `env` variable per line, but please do not tab-align > the variable (`bool_env`, `integer_env`, `double_env`) definition (use > one space before the `=` and one after). > Done. >> + 'ERRINJ_WAL_IO=True ' .. >> +                        'ERRINJ_WAL_SYNC=TRUE ' .. >> +                        'ERRINJ_WAL_WRITE=false ' .. >> +                        'ERRINJ_INDEX_ALLOC=False ' .. >> +                        'ERRINJ_WAL_WRITE_DISK=FALSE' >> +local integer_env  =    'ERRINJ_WAL_WRITE_PARTIAL=2 ' .. >> +                        'ERRINJ_WAL_FALLOCATE=+2 ' .. >> +                        'ERRINJ_WAL_WRITE_COUNT=-2' >> +local double_env   =    'ERRINJ_VY_READ_PAGE_TIMEOUT=2.5 ' .. >> +                        'ERRINJ_VY_SCHED_TIMEOUT=+2.5 ' .. >> +                        'ERRINJ_RELAY_TIMEOUT=-2.5' >> +local set_env_str = ('%s %s %s'):format(bool_env, integer_env, >> double_env) >> +local path_to_test_file = fio.pathjoin( >> +        os.getenv('PWD'), >> +        'box-tap', >> +        'errinj_set_with_enviroment_vars_script.lua') >> +local shell_command = ('%s %s %s'):format( >> +                set_env_str, >> +                TARANTOOL_PATH, >> +                path_to_test_file) >> + >> +os.exit(os.execute(shell_command)) >> diff --git a/test/box-tap/errinj_set_with_enviroment_vars_script.lua >> b/test/box-tap/errinj_set_with_enviroment_vars_script.lua >> new file mode 100644 >> index 000000000..f15085aa0 >> --- /dev/null >> +++ b/test/box-tap/errinj_set_with_enviroment_vars_script.lua >> @@ -0,0 +1,34 @@ >> +-- Script for box-tap/errinj_set_with_enviroment_vars.test.lua test. >> + >> +local tap = require('tap') >> +local errinj = box.error.injection >> + >> +local test = tap.test('set errinjs via environment variables') >> + >> +test:plan(3) >> + >> +test:test('Set boolean error injections', function(test) >> +    test:plan(6) >> +    test:is(errinj.get('ERRINJ_TESTING'), true, 'true') >> +    test:is(errinj.get('ERRINJ_WAL_IO'), true, 'True') >> +    test:is(errinj.get('ERRINJ_WAL_SYNC'), true, 'TRUE') >> +    test:is(errinj.get('ERRINJ_WAL_WRITE'), false, 'false') >> +    test:is(errinj.get('ERRINJ_INDEX_ALLOC'), false, 'False') >> +    test:is(errinj.get('ERRINJ_WAL_WRITE_DISK'), false, 'FALSE') >> +end) >> + >> +test:test('Set integer error injections', function(test) >> +    test:plan(3) >> +    test:is(errinj.get('ERRINJ_WAL_WRITE_PARTIAL'), 2, '2') >> +    test:is(errinj.get('ERRINJ_WAL_FALLOCATE'), 2, '+2') >> +    test:is(errinj.get('ERRINJ_WAL_WRITE_COUNT'), -2, '-2') >> +end) >> + >> +test:test('Set double error injections', function(test) >> +    test:plan(3) >> +    test:is(errinj.get('ERRINJ_VY_READ_PAGE_TIMEOUT'), 2.5, "2.5") >> +    test:is(errinj.get('ERRINJ_VY_SCHED_TIMEOUT'), 2.5, "+2.5") >> +    test:is(errinj.get('ERRINJ_RELAY_TIMEOUT'), -2.5, "-2.5") >> +end) >> + >> +os.exit(test:check() and 0 or 1) >> diff --git a/test/box-tap/suite.ini b/test/box-tap/suite.ini >> index a7c03baa8..8ed7fffb9 100644 >> --- a/test/box-tap/suite.ini >> +++ b/test/box-tap/suite.ini >> @@ -4,6 +4,7 @@ description = Database tests with #! using TAP >>   is_parallel = True >>   pretest_clean = True >>   use_unix_sockets_iproto = True >> +release_disabled = errinj_set_with_enviroment_vars.test.lua >>   config = suite.cfg >>   fragile = { >>       "retries": 10, >>