[Tarantool-patches] [PATCH 2/2] vshard: fix buckets_count() on replicas

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Fri May 21 22:30:34 MSK 2021


Thanks for the review!

>     diff --git a/test/misc/reconfigure.result b/test/misc/reconfigure.result
>     index 3b34841..a03ab5a 100644
>     --- a/test/misc/reconfigure.result
>     +++ b/test/misc/reconfigure.result
>     @@ -271,9 +271,9 @@ box.cfg.read_only
>      ---
>      - false
>      ...
>     -#box.space._bucket:on_replace()
>     +assert(#box.space._bucket:on_replace() == 1)
>      ---
>     -- 1
>     +- true
>      ...
> 
> 
> Why is this change necessary? Seems now you miss an actual value in case the test fails.
> This question applies to many cases below.

See me response in the first email about the assertions.

> No matter what testing framework is used, I think that
> 
> assert_equals(value, 1) -- assertion failed: expected 1, got 2
> 
> is better (more helpful) than eager
> 
> assert(value == 1) -- assertion failed
Yes, but what you are describing is present in the tap tests
framework. Which currently is hardly usable for complex multi-instance
tests. Hence I use what is available. Decided not to implement my
own testing suite for these checks as it is anyway trivial to
get more info if they start failing.

Until tap would be easier to use for multi-instance tests.

>     @@ -2612,17 +2676,15 @@ local function storage_cfg(cfg, this_replica_uuid, is_reload)
>          local uri = luri.parse(this_replica.uri)
>          schema_upgrade(is_master, uri.login, uri.password)
> 
>     -    if M.bucket_on_replace then
>     -        box.space._bucket:on_replace(nil, M.bucket_on_replace)
>     -        M.bucket_on_replace = nil
>     -    end
>     -    lref.cfg()
>     -    if is_master then
>     -        box.space._bucket:on_replace(bucket_generation_increment)
>     -        M.bucket_on_replace = bucket_generation_increment
>     +    if is_master or box.space._bucket then
>     +        schema_install_triggers()
> 
> 
> +1 to Oleg, isn't _bucket check enough?

I want to emphasize that on the master the bootstrap is finished anyway,
because it performs the bootstrap. On the master no need to check for
_bucket nor wait for the schema anyhow.


More information about the Tarantool-patches mailing list