Re: [Tarantool-patches] [PATCH vshard 1/5] router: backoff on some box errors

[Oleg Babin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thanks for your fixes. Yes, it is that I meant. LGTM.

On 18.12.2021 02:10, Vladislav Shpilevoy wrote:
> Hi! Thanks for the review!
>
> On 17.12.2021 12:09, Oleg Babin wrote:
>> Thanks for your patch! LGTM. But I think tests should be extended a bit.
>>
>> I see that tests cover AccessDenied error but I don't see that there is simple error()/assert() error check.
> I hope you meant this:
>
> ====================
> diff --git a/test/router/router2.result b/test/router/router2.result
> index a501dbf..85d077a 100644
> --- a/test/router/router2.result
> +++ b/test/router/router2.result
> @@ -548,6 +548,28 @@ vshard.storage.call = old_storage_call
>    | ---
>    | ...
>   
> +--
> +-- Fails without backoff for other errors.
> +--
> +test_run:switch('router_1')
> + | ---
> + | - true
> + | ...
> +fiber.sleep(vshard.consts.REPLICA_BACKOFF_INTERVAL)
> + | ---
> + | ...
> +rs = vshard.router.route(1)
> + | ---
> + | ...
> +ok, err = rs:callro('vshard.storage.call', {1, 'badmode', 'echo', {100}},       \
> +                    long_timeout)
> + | ---
> + | ...
> +assert(not ok and err.message:match('Unknown mode') ~= nil)
> + | ---
> + | - true
> + | ...
> +
>   _ = test_run:switch("default")
>    | ---
>    | ...
> diff --git a/test/router/router2.test.lua b/test/router/router2.test.lua
> index fb0c3b2..a2171fa 100644
> --- a/test/router/router2.test.lua
> +++ b/test/router/router2.test.lua
> @@ -216,6 +216,16 @@ test_run:switch('storage_2_a')
>   assert(echo_count == 0)
>   vshard.storage.call = old_storage_call
>   
> +--
> +-- Fails without backoff for other errors.
> +--
> +test_run:switch('router_1')
> +fiber.sleep(vshard.consts.REPLICA_BACKOFF_INTERVAL)
> +rs = vshard.router.route(1)
> +ok, err = rs:callro('vshard.storage.call', {1, 'badmode', 'echo', {100}},       \
> +                    long_timeout)
> +assert(not ok and err.message:match('Unknown mode') ~= nil)
> +
>   _ = test_run:switch("default")
>   _ = test_run:cmd("stop server router_1")
>   _ = test_run:cmd("cleanup server router_1")