From: Alex Khatskevich <avkhatskevich@tarantool.org> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>, tarantool-patches@freelists.org Subject: [tarantool-patches] Re: [PATCH 1/2] Add test on error during reconfigure Date: Mon, 18 Jun 2018 13:10:54 +0300 [thread overview] Message-ID: <a27321e7-73c9-c335-9da5-a51c1ffe9fd4@tarantool.org> (raw) In-Reply-To: <1fc9755e-0adc-8a44-4016-dedb7ba557b7@tarantool.org> >> +-- Error during reconfigure process. >> +_ = vshard.router.route(1):callro('echo', {'some_data'}) > > 1. Why do you need this call here? It does nor outputs > nothing. Because next call checks a raised error. This line ensures that an error ocured only after `vshard.router.internal.errinj.ERRINJ_CFG = true` just by not failing. I did it to prevent test.result flooding. >> +--- >> +... >> +vshard.router.internal.errinj.ERRINJ_CFG = true >> +--- >> +... >> +old_internal = table.copy(vshard.router.internal) >> +--- >> +... >> +_, err = pcall(vshard.router.cfg, cfg) >> +--- > 2. Same. Maybe this call should output something? This > test would not fail if callro failed. Call/ro/rw do not > throw exceptions but return a pair: result and error if > occurred. The error I test here returns as a raised exception. > 3. Same. Here and at the end of the test. Same >> diff --git a/vshard/router/init.lua b/vshard/router/init.lua >> index 21093e5..1dee80c 100644 >> --- a/vshard/router/init.lua >> +++ b/vshard/router/init.lua >> @@ -473,6 +474,12 @@ local function router_cfg(cfg) >> end >> box.cfg(cfg) >> log.info("Box has been configured") >> + -- It is considered that all possible errors during cfg >> + -- process occur only before this place. >> + -- This check should be placed as late as possible. >> + if M.errinj.ERRINJ_CFG then >> + error('Error injection: cfg') >> + end > > 4. I think, you should place this injection before box.cfg. > All the cfg() code relies on the fact, that box.cfg is atomic. > And if box.cfg is ok, then storage/router.cfg finish with > no errors. An error is possible only before box.cfg and in > box.cfg. You may think box.cfg like commit, and code before > box.cfg like prepare. In this case it does not matter, because it is just an errinj. However I wanted to show here, that error should be raised as late as possible. It can help in the future, if someone adds a code after `box.cfg`. I prefer errnj to be here and will move it only if you insist.
next prev parent reply other threads:[~2018-06-18 10:10 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-06-09 17:47 [tarantool-patches] [PATCH 0/2][vshard] Vshard safe reload AKhatskevich 2018-06-09 17:47 ` [tarantool-patches] [PATCH 1/2] Add test on error during reconfigure AKhatskevich 2018-06-17 19:46 ` [tarantool-patches] " Vladislav Shpilevoy 2018-06-18 10:10 ` Alex Khatskevich [this message] 2018-06-18 10:48 ` Vladislav Shpilevoy 2018-06-19 10:36 ` Alex Khatskevich 2018-06-09 17:47 ` [tarantool-patches] [PATCH 2/2] Complete module reload AKhatskevich 2018-06-17 19:46 ` [tarantool-patches] " Vladislav Shpilevoy 2018-06-19 7:09 ` Alex Khatskevich 2018-06-19 9:00 ` Vladislav Shpilevoy 2018-06-19 11:49 ` Alex Khatskevich 2018-06-19 11:57 ` Vladislav Shpilevoy 2018-06-17 19:39 ` [tarantool-patches] Re: [PATCH 0/2][vshard] Vshard safe reload Vladislav Shpilevoy 2018-06-19 7:20 ` Alex Khatskevich
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=a27321e7-73c9-c335-9da5-a51c1ffe9fd4@tarantool.org \ --to=avkhatskevich@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=v.shpilevoy@tarantool.org \ --subject='[tarantool-patches] Re: [PATCH 1/2] Add test on error during reconfigure' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox