[Tarantool-patches] [PATCH test-run 1/1] Wait_upstream/downstream should be ready to nil info

[Tarantool-patches] [PATCH test-run 1/1] Wait_upstream/downstream should be ready to nil info

[Vladislav Shpilevoy]

Wait_upstream/downstream are helper functions of a
test-run object. They are used to wait until a certain
state of upstream/downstream is reached such as error
message, replication status.

They take these attributes from
box.info.replication[id].upstream/downstream, which
sometimes can be nil, and it led to an error.

Now these waiters check upstream/downstream for nil.

The bug was discovered in Tarantool in
replication/show_error_on_disconnect test:
https://github.com/tarantool/tarantool/issues/4563.

It appeared, because after my patch reconfiguration of
replication with 0 quorum always returns immediately.
And right after the reconfiguration the
upstream/downstream may still do not exist. This is
why test_run:wait_upstream/downstream might fail like
this:

    [002]  test_run:wait_upstream(other_id, {status = 'stopped', message_re = 'Missing'})
    [002]  ---
    [002] -- true
    [002] +- error: '.../Workspaces/gitlab/test/var/002_replication/test_run.lua:250: attempt
    [002] +    to index local ''info'' (a nil value)'
---
Branch: https://github.com/tarantool/test-run/tree/gerold103/gh-tarantool-4563-box.info.repl.upstream-nil
Issue: https://github.com/tarantool/tarantool/issues/4563

 test_run.lua | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/test_run.lua b/test_run.lua
index 8f13491..09d53af 100644
--- a/test_run.lua
+++ b/test_run.lua
@@ -234,10 +234,15 @@ local function log_box_info_replication_cond(id, field, ok, info, opts)
         status = opts.status,
         message_re = opts.message_re,
     })
-    local got = json.encode({
-        status = info.status,
-        message = info.message,
-    })
+    local got
+    if info then
+        got = json.encode({
+            status = info.status,
+            message = info.message,
+        })
+    else
+        got = json.encode('info is nil')
+    end
     log.info('wait_%s(%d, ...); exp = %s; got = %s; result = %s', field, id,
         exp, got, tostring(ok))
 end
@@ -247,7 +252,7 @@ local function gen_box_info_replication_cond(id, field, opts)
         local info = box.info.replication[id][field]
         local ok = true
         if opts.status ~= nil then
-            ok = ok and info.status == opts.status
+            ok = ok and info and info.status == opts.status
         end
         if opts.message_re ~= nil then
             -- regex match
-- 
2.21.0 (Apple Git-122)

Re: [Tarantool-patches] [PATCH test-run 1/1] Wait_upstream/downstream should be ready to nil info

[Alexander Turenko]

Aren't you mind against those fixes (they are mostly just my
preferences):

 | diff --git a/test_run.lua b/test_run.lua
 | index 09d53af..63dfdef 100644
 | --- a/test_run.lua
 | +++ b/test_run.lua
 | @@ -234,15 +234,10 @@ local function log_box_info_replication_cond(id, field, ok, info, opts)
 | 		 status = opts.status,
 | 		 message_re = opts.message_re,
 | 	 })
 | -    local got
 | -    if info then
 | -        got = json.encode({
 | -            status = info.status,
 | -            message = info.message,
 | -        })
 | -    else
 | -        got = json.encode('info is nil')
 | -    end
 | +    local got = json.encode(info ~= nil and {
 | +        status = info.status,
 | +        message = info.message,
 | +    } or nil)
 | 	 log.info('wait_%s(%d, ...); exp = %s; got = %s; result = %s', field, id,
 | 		 exp, got, tostring(ok))
 |  end
 | @@ -250,9 +245,9 @@ end
 |  local function gen_box_info_replication_cond(id, field, opts)
 | 	 return function()
 | 		 local info = box.info.replication[id][field]
 | -        local ok = true
 | +        local ok = info ~= nil
 | 		 if opts.status ~= nil then
 | -            ok = ok and info and info.status == opts.status
 | +            ok = ok and info.status == opts.status
 | 		 end
 | 		 if opts.message_re ~= nil then
 | 			 -- regex match
 | @@ -270,6 +265,10 @@ end
 |  
 |  --- Wait for upstream status.
 |  ---
 | +--- The function waits until information about an upstream with
 | +--- provided id will appear (regardless of passed options) and
 | +--- then waits for a certain state of the upstream if requested.
 | +---
 |  --- If `opts.status` is `nil` or `box.NULL` an upstream status is
 |  --- not checked.
 |  ---

I pushed them on top of your branch as a separate commit. Squash if you are
agree.

WBR, Alexander Turenko.

On Sun, Oct 13, 2019 at 05:09:28PM +0200, Vladislav Shpilevoy wrote:
> Wait_upstream/downstream are helper functions of a
> test-run object. They are used to wait until a certain
> state of upstream/downstream is reached such as error
> message, replication status.
> 
> They take these attributes from
> box.info.replication[id].upstream/downstream, which
> sometimes can be nil, and it led to an error.
> 
> Now these waiters check upstream/downstream for nil.
> 
> The bug was discovered in Tarantool in
> replication/show_error_on_disconnect test:
> https://github.com/tarantool/tarantool/issues/4563.
> 
> It appeared, because after my patch reconfiguration of
> replication with 0 quorum always returns immediately.
> And right after the reconfiguration the
> upstream/downstream may still do not exist. This is
> why test_run:wait_upstream/downstream might fail like
> this:
> 
>     [002]  test_run:wait_upstream(other_id, {status = 'stopped', message_re = 'Missing'})
>     [002]  ---
>     [002] -- true
>     [002] +- error: '.../Workspaces/gitlab/test/var/002_replication/test_run.lua:250: attempt
>     [002] +    to index local ''info'' (a nil value)'
> ---
> Branch: https://github.com/tarantool/test-run/tree/gerold103/gh-tarantool-4563-box.info.repl.upstream-nil
> Issue: https://github.com/tarantool/tarantool/issues/4563
> 
>  test_run.lua | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/test_run.lua b/test_run.lua
> index 8f13491..09d53af 100644
> --- a/test_run.lua
> +++ b/test_run.lua
> @@ -234,10 +234,15 @@ local function log_box_info_replication_cond(id, field, ok, info, opts)
>          status = opts.status,
>          message_re = opts.message_re,
>      })
> -    local got = json.encode({
> -        status = info.status,
> -        message = info.message,
> -    })
> +    local got
> +    if info then
> +        got = json.encode({
> +            status = info.status,
> +            message = info.message,
> +        })
> +    else
> +        got = json.encode('info is nil')
> +    end
>      log.info('wait_%s(%d, ...); exp = %s; got = %s; result = %s', field, id,
>          exp, got, tostring(ok))
>  end
> @@ -247,7 +252,7 @@ local function gen_box_info_replication_cond(id, field, opts)
>          local info = box.info.replication[id][field]
>          local ok = true
>          if opts.status ~= nil then
> -            ok = ok and info.status == opts.status
> +            ok = ok and info and info.status == opts.status
>          end
>          if opts.message_re ~= nil then
>              -- regex match
> -- 
> 2.21.0 (Apple Git-122)
> 

Re: [Tarantool-patches] [tarantool-patches] Re: [PATCH test-run 1/1] Wait_upstream/downstream should be ready to nil info

[Vladislav Shpilevoy]

Hi! Thanks for the review!

The changes are ok, I squashed them.

Re: [Tarantool-patches] [tarantool-patches] Re: [PATCH test-run 1/1] Wait_upstream/downstream should be ready to nil info

[Alexander Turenko]

On Tue, Oct 15, 2019 at 09:09:50PM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the review!
> 
> The changes are ok, I squashed them.

LGTM.

Pushed to test-run's master, updated the submodule in master, 2.2, 2.1,
1.10.

WBR, Alexander Turenko.