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 42CFC6EC5F; Tue, 2 Mar 2021 20:00:32 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 42CFC6EC5F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1614704432; bh=/hBscsF0wwJdIiWiOqw1tTa1/LPSw1Yl3Rd1Q5hkeD0=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=TOzwLjo5xoo1rQFi1f5J/i+zmfMEqp9jYmP8ap4N8HN2KbqFY5PHqp6RxsHPSHavK g7ci/VwhNto5R5Al3GSovRT7nYM6+QvAmx8GJloMq1EOtfbX85TaiubHWv7jWbe5ov 9M0LZNDdeZtm6+Yk+V3MI7lU1uZg3NmIGweBPjFU= Received: from smtp49.i.mail.ru (smtp49.i.mail.ru [94.100.177.109]) (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 95D246EC5F for ; Tue, 2 Mar 2021 20:00:29 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 95D246EC5F Received: by smtp49.i.mail.ru with esmtpa (envelope-from ) id 1lH8No-0005Ph-5I; Tue, 02 Mar 2021 20:00:28 +0300 To: Leonid Vasiliev , Alexander Turenko , Sergey Bronnikov Cc: tarantool-patches@dev.tarantool.org References: <7a1f0cb7a97b41b3cde25ac5d2c591a84aa13f0b.1613674486.git.artemreyt@tarantool.org> <1493e11a-e8ca-6dc0-c4cf-7f8bb4dd0d71@tarantool.org> Message-ID: Date: Tue, 2 Mar 2021 20:00:27 +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: <1493e11a-e8ca-6dc0-c4cf-7f8bb4dd0d71@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: ru X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD92A98208ECBDD29F51A014079BC0C7C459F7160AC7246F773182A05F538085040D01A6FD56E042BEA901D78242696F9580AA98D17C8F011D662B0962BFECD1354 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7F12ABE79F2AB44EAEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637BA14B6CBCF09CA1D8638F802B75D45FF914D58D5BE9E6BC131B5C99E7648C95CDB1C6D2319E1F167EC7ED2F421C344D3F093A5BAA3F4B830A471835C12D1D9774AD6D5ED66289B5278DA827A17800CE77A825AB47F0FC8649FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C327ED053E960B195E117882F4460429728AD0CFFFB425014E868A13BD56FB6657D81D268191BDAD3DC09775C1D3CA48CF2D001AC7ACD80EC4BA3038C0950A5D36C8A9BA7A39EFB766EC990983EF5C0329BA3038C0950A5D36D5E8D9A59859A8B6625D8AB6E1FD60573AA81AA40904B5D9DBF02ECDB25306B2B25CBF701D1BE8734AD6D5ED66289B5278DA827A17800CE75B51C8FB0C3E748C67F23339F89546C5A8DF7F3B2552694A6FED454B719173D6725E5C173C3A84C38E7B408EF846B9DA35872C767BF85DA2F004C906525384306FED454B719173D6462275124DF8B9C9DE2850DD75B2526BE5BFE6E7EFDEDCD789D4C264860C145E X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A24A6D60772A99906F8E1CD14B953EB46DB010A48F8FD08C51355D89D7DBCDD132 X-C1DE0DAB: 0D63561A33F958A5C4F2E145811D7FA0F936461442D68C102169335CC14F3FD1D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75448CF9D3A7B2C848410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34181D1E89D5A0B42F9632AA3CB15FC695A3F26FEF97EEAD528BD3AA954F0DC4944948F053FB1FF8781D7E09C32AA3244CB356FC3F85F57770A1FA4517A33845D181560E2432555DBB927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojnllmSZCgRqu8waOk0F2soA== X-Mailru-Sender: 65CD03CFE33A0EC7DB9F8DC6996C930753BFE6C0ADD854E60130FD51271149148A3B2647B1303A8A92F02B39BF137DED0A8ED71B308007E347E5C1256AE813B3E2FBE92986FDCCC89437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCHv2 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 Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hello! Thanks for your review! Branch is updated. See my 'DONE's below) 01.03.2021 17:47, Leonid Vasiliev пишет: > HI! Thank you for the patch. > Seems like write a test is more difficult than fixing the bug) It is what it is... > Generally OK. > I'll paste the last variation of the diff with comments. > > > From c2ff4adb95eda297e61ed6836f391a541ab08fd3 Mon Sep 17 00:00:00 2001 > Message-Id: > > In-Reply-To: > References: > From: Artem Starshov > Date: Tue, 26 Jan 2021 20:15:22 +0300 > Subject: [PATCH 1/2] core: add setting error injections via env > Cc: tarantool-patches@dev.tarantool.org > > 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". > > Part of #5040 > --- >  src/lib/core/errinj.c                         | 31 +++++++++++++++++ >  src/lib/core/errinj.h                         |  5 +++ >  src/main.cc                                   |  1 + >  .../errinj_set_with_enviroment_vars.test.lua  | 18 ++++++++++ >  ...errinj_set_with_enviroment_vars_script.lua | 34 +++++++++++++++++++ >  5 files changed, 89 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..983ead681 100644 > --- a/src/lib/core/errinj.c > +++ b/src/lib/core/errinj.c > @@ -28,6 +28,7 @@ >   * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF >   * SUCH DAMAGE. >   */ > +#include >  #include >  #include >  #include > @@ -66,3 +67,33 @@ int errinj_foreach(errinj_cb cb, void *cb_ctx) { >      } >      return 0; >  } > + > +void errinj_set_with_environment_vars() { > > Comment: > void errinj_set_with_environment_vars(void) see > https://github.com/tarantool/tarantool/issues/4718 1. Done. > + 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 || *env_value == '\0') > > Comment: > env_value == NULL > (https://github.com/tarantool/tarantool/wiki/Code-review-procedure#code-style) 2. Done. > + continue; > + > +        if (inj->type == ERRINJ_INT) { > +            char *end; > +            int64_t int_value = strtoll(env_value, &end, 10); > +            if (*end == '\0') > +                inj->iparam = int_value; > > Comment: > Seems like the `panic()` should be called in the case of `else()`. > Here and in > other cases of parsing. 3. Done. > > +        } else if (inj->type == ERRINJ_BOOL) { > +            char *lower_env_value = (char *)calloc(strlen(env_value) > + 1, sizeof(char)); > > Comment: > `(char *)` - unneeded in C. > Maximal width of a code line is 80 symbols. 4. Done. > + for (size_t i = 0; env_value[i]; ++i) > +                lower_env_value[i] = tolower(env_value[i]); > > Comment: > I suggest to use `strcasecmp()`. 5. Nice suggestion! Done. > + if (strcmp(lower_env_value, "false") == 0) > +                inj->bparam = false; > +            else if (strcmp(lower_env_value, "true") == 0) > +                inj->bparam = true; > +            free(lower_env_value); > +        } else if (inj->type == ERRINJ_DOUBLE) { > +            char *end; > +            double double_value = strtod(env_value, &end); > +            if (*end == '\0') > +                inj->dparam = double_value; > +        } > +    } > +} > diff --git a/src/lib/core/errinj.h b/src/lib/core/errinj.h > index 814c57c2e..d76aedf7a 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(); > + >  #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..58a660689 100644 > --- a/src/main.cc > +++ b/src/main.cc > @@ -710,6 +710,7 @@ main(int argc, char **argv) >      memtx_tx_manager_init(); >      crypto_init(); >      systemd_init(); > +    errinj_set_with_environment_vars(); >      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..bffcc31e7 > --- /dev/null > +++ b/test/box-tap/errinj_set_with_enviroment_vars.test.lua > @@ -0,0 +1,18 @@ > +#!/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 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' > > Comment: > Maximal width of a code line is 80 symbols. 6. Done. > +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)