Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH] Fix flaky test engine/ddl
@ 2020-04-10 13:27 Sergey Bronnikov
  2020-04-11 12:55 ` Serge Petrenko
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Sergey Bronnikov @ 2020-04-10 13:27 UTC (permalink / raw)
  To: sergepetrenko, tarantool-patches, avtikhon, o.piskunov

Test was a flaky from the beginning 39d0e4273dde2dbb3e46aea35310379e98e7cc64
Time of building indexes varies from time to time and the problem was due to
abcense of synchronization in index building and checking numbers of these
indexes.

Fixes #4353
---
 test/engine/ddl.result   | 15 +++++++++------
 test/engine/ddl.test.lua | 14 ++++++++------
 test/engine/suite.ini    |  3 +--
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/test/engine/ddl.result b/test/engine/ddl.result
index 67b22ed9e..6cf429c9b 100644
--- a/test/engine/ddl.result
+++ b/test/engine/ddl.result
@@ -2461,20 +2461,23 @@ ch:get()
 ---
 - true
 ...
-box.space.test.index.pk:count() == box.space.test.index.sk:count()
+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
 ---
 - true
 ...
-box.space.test.index.pk:count() == box.space.test.index.tk:count()
+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
 ---
 - true
 ...
 inspector:cmd("restart server default")
-box.space.test.index.pk:count() == box.space.test.index.sk:count()
+inspector = require('test_run').new()
+---
+...
+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
 ---
 - true
 ...
-box.space.test.index.pk:count() == box.space.test.index.tk:count()
+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
 ---
 - true
 ...
@@ -2482,11 +2485,11 @@ box.snapshot()
 ---
 - ok
 ...
-box.space.test.index.pk:count() == box.space.test.index.sk:count()
+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
 ---
 - true
 ...
-box.space.test.index.pk:count() == box.space.test.index.tk:count()
+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
 ---
 - true
 ...
diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua
index e761966d7..57f1beb03 100644
--- a/test/engine/ddl.test.lua
+++ b/test/engine/ddl.test.lua
@@ -976,15 +976,17 @@ _ = fiber.create(function() gen_load() ch:put(true) end)
 _ = box.space.test:create_index('tk', {unique = true, parts = {3, 'unsigned'}})
 ch:get()
 
-box.space.test.index.pk:count() == box.space.test.index.sk:count()
-box.space.test.index.pk:count() == box.space.test.index.tk:count()
+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
 
 inspector:cmd("restart server default")
 
-box.space.test.index.pk:count() == box.space.test.index.sk:count()
-box.space.test.index.pk:count() == box.space.test.index.tk:count()
+inspector = require('test_run').new()
+
+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
 box.snapshot()
-box.space.test.index.pk:count() == box.space.test.index.sk:count()
-box.space.test.index.pk:count() == box.space.test.index.tk:count()
+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
 
 box.space.test:drop()
diff --git a/test/engine/suite.ini b/test/engine/suite.ini
index 5ae12a431..e78b8c261 100644
--- a/test/engine/suite.ini
+++ b/test/engine/suite.ini
@@ -10,5 +10,4 @@ config = engine.cfg
 lua_libs = conflict.lua ../box/lua/utils.lua ../box/lua/push.lua
 is_parallel = True
 pretest_clean = True
-fragile = ddl.test.lua         ; gh-4353
-          recover_wal.test.lua ; gh-3767
+fragile = recover_wal.test.lua ; gh-3767
-- 
2.23.0


-- 
sergeyb@

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Tarantool-patches] [PATCH] Fix flaky test engine/ddl
  2020-04-10 13:27 [Tarantool-patches] [PATCH] Fix flaky test engine/ddl Sergey Bronnikov
@ 2020-04-11 12:55 ` Serge Petrenko
  2020-04-13 13:06   ` Sergey Bronnikov
  2020-04-13  8:45 ` Oleg Piskunov
  2020-04-15 11:14 ` Kirill Yukhin
  2 siblings, 1 reply; 5+ messages in thread
From: Serge Petrenko @ 2020-04-11 12:55 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: o.piskunov, tml

Hi! Thanks for the patch!


> 10 апр. 2020 г., в 16:27, Sergey Bronnikov <sergeyb@tarantool.org> написал(а):
> 
> Test was a flaky from the beginning 39d0e4273dde2dbb3e46aea35310379e98e7cc64
> Time of building indexes varies from time to time and the problem was due to
> abcense of synchronization in index building and checking numbers of these
> indexes.
> 
> Fixes #4353
> ---

Please attach links to the issue and branch after `---` next time.

> test/engine/ddl.result   | 15 +++++++++------
> test/engine/ddl.test.lua | 14 ++++++++------
> test/engine/suite.ini    |  3 +--
> 3 files changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/test/engine/ddl.result b/test/engine/ddl.result
> index 67b22ed9e..6cf429c9b 100644
> --- a/test/engine/ddl.result
> +++ b/test/engine/ddl.result
> @@ -2461,20 +2461,23 @@ ch:get()
> ---
> - true
> ...
> -box.space.test.index.pk:count() == box.space.test.index.sk:count()
> +inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
> ---
> - true
> ...
> -box.space.test.index.pk:count() == box.space.test.index.tk:count()
> +inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
> ---
> - true
> ...
> inspector:cmd("restart server default")
> -box.space.test.index.pk:count() == box.space.test.index.sk:count()
> +inspector = require('test_run').new()
> +---
> +...
> +inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
> ---
> - true
> ...
> -box.space.test.index.pk:count() == box.space.test.index.tk:count()
> +inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
> ---
> - true
> ...
> @@ -2482,11 +2485,11 @@ box.snapshot()
> ---
> - ok
> ...
> -box.space.test.index.pk:count() == box.space.test.index.sk:count()
> +inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
> ---
> - true
> ...
> -box.space.test.index.pk:count() == box.space.test.index.tk:count()
> +inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
> ---
> - true
> ...
> diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua
> index e761966d7..57f1beb03 100644
> --- a/test/engine/ddl.test.lua
> +++ b/test/engine/ddl.test.lua
> @@ -976,15 +976,17 @@ _ = fiber.create(function() gen_load() ch:put(true) end)
> _ = box.space.test:create_index('tk', {unique = true, parts = {3, 'unsigned'}})
> ch:get()
> 
> -box.space.test.index.pk:count() == box.space.test.index.sk:count()
> -box.space.test.index.pk:count() == box.space.test.index.tk:count()
> +inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
> +inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
> 
> inspector:cmd("restart server default")
> 
> -box.space.test.index.pk:count() == box.space.test.index.sk:count()
> -box.space.test.index.pk:count() == box.space.test.index.tk:count()
> +inspector = require('test_run').new()
> +
> +inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
> +inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
> box.snapshot()
> -box.space.test.index.pk:count() == box.space.test.index.sk:count()
> -box.space.test.index.pk:count() == box.space.test.index.tk:count()
> +inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
> +inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)

I think the check can’t fail after `box.snapshot()` even without wait_cond().
You’ve already waited for the index to build right before `box.snapshot()`.
It’ s better to leave the last 2 checks without wait_cond to emphasize that
nothing could happen during `box.snapshot()`.

> 
> box.space.test:drop()
> diff --git a/test/engine/suite.ini b/test/engine/suite.ini
> index 5ae12a431..e78b8c261 100644
> --- a/test/engine/suite.ini
> +++ b/test/engine/suite.ini
> @@ -10,5 +10,4 @@ config = engine.cfg
> lua_libs = conflict.lua ../box/lua/utils.lua ../box/lua/push.lua
> is_parallel = True
> pretest_clean = True
> -fragile = ddl.test.lua         ; gh-4353
> -          recover_wal.test.lua ; gh-3767
> +fragile = recover_wal.test.lua ; gh-3767
> -- 
> 2.23.0
> 
> 
> -- 
> sergeyb@


--
Serge Petrenko
sergepetrenko@tarantool.org

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Tarantool-patches] [PATCH] Fix flaky test engine/ddl
  2020-04-10 13:27 [Tarantool-patches] [PATCH] Fix flaky test engine/ddl Sergey Bronnikov
  2020-04-11 12:55 ` Serge Petrenko
@ 2020-04-13  8:45 ` Oleg Piskunov
  2020-04-15 11:14 ` Kirill Yukhin
  2 siblings, 0 replies; 5+ messages in thread
From: Oleg Piskunov @ 2020-04-13  8:45 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 4207 bytes --]


LGTM

  
>Пятница, 10 апреля 2020, 16:27 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:
> 
>Test was a flaky from the beginning 39d0e4273dde2dbb3e46aea35310379e98e7cc64
>Time of building indexes varies from time to time and the problem was due to
>abcense of synchronization in index building and checking numbers of these
>indexes.
>
>Fixes #4353
>---
> test/engine/ddl.result | 15 +++++++++------
> test/engine/ddl.test.lua | 14 ++++++++------
> test/engine/suite.ini | 3 +--
> 3 files changed, 18 insertions(+), 14 deletions(-)
>
>diff --git a/test/engine/ddl.result b/test/engine/ddl.result
>index 67b22ed9e..6cf429c9b 100644
>--- a/test/engine/ddl.result
>+++ b/test/engine/ddl.result
>@@ -2461,20 +2461,23 @@ ch:get()
> ---
> - true
> ...
>-box.space.test.index.pk:count() == box.space.test.index.sk:count()
>+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
> ---
> - true
> ...
>-box.space.test.index.pk:count() == box.space.test.index.tk:count()
>+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
> ---
> - true
> ...
> inspector:cmd("restart server default")
>-box.space.test.index.pk:count() == box.space.test.index.sk:count()
>+inspector = require('test_run').new()
>+---
>+...
>+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
> ---
> - true
> ...
>-box.space.test.index.pk:count() == box.space.test.index.tk:count()
>+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
> ---
> - true
> ...
>@@ -2482,11 +2485,11 @@ box.snapshot()
> ---
> - ok
> ...
>-box.space.test.index.pk:count() == box.space.test.index.sk:count()
>+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
> ---
> - true
> ...
>-box.space.test.index.pk:count() == box.space.test.index.tk:count()
>+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
> ---
> - true
> ...
>diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua
>index e761966d7..57f1beb03 100644
>--- a/test/engine/ddl.test.lua
>+++ b/test/engine/ddl.test.lua
>@@ -976,15 +976,17 @@ _ = fiber.create(function() gen_load() ch:put(true) end)
> _ = box.space.test:create_index('tk', {unique = true, parts = {3, 'unsigned'}})
> ch:get()
> 
>-box.space.test.index.pk:count() == box.space.test.index.sk:count()
>-box.space.test.index.pk:count() == box.space.test.index.tk:count()
>+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
>+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
> 
> inspector:cmd("restart server default")
> 
>-box.space.test.index.pk:count() == box.space.test.index.sk:count()
>-box.space.test.index.pk:count() == box.space.test.index.tk:count()
>+inspector = require('test_run').new()
>+
>+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
>+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
> box.snapshot()
>-box.space.test.index.pk:count() == box.space.test.index.sk:count()
>-box.space.test.index.pk:count() == box.space.test.index.tk:count()
>+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.sk:count() end)
>+inspector:wait_cond(function() return box.space.test.index.pk:count() == box.space.test.index.tk:count() end)
> 
> box.space.test:drop()
>diff --git a/test/engine/suite.ini b/test/engine/suite.ini
>index 5ae12a431..e78b8c261 100644
>--- a/test/engine/suite.ini
>+++ b/test/engine/suite.ini
>@@ -10,5 +10,4 @@ config = engine.cfg
> lua_libs = conflict.lua ../box/lua/utils.lua ../box/lua/push.lua
> is_parallel = True
> pretest_clean = True
>-fragile = ddl.test.lua ; gh-4353
>- recover_wal.test.lua ; gh-3767
>+fragile = recover_wal.test.lua ; gh-3767
>--
>2.23.0
>
>
>--
>sergeyb@ 
 
 
--
Oleg Piskunov
 

[-- Attachment #2: Type: text/html, Size: 5009 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Tarantool-patches] [PATCH] Fix flaky test engine/ddl
  2020-04-11 12:55 ` Serge Petrenko
@ 2020-04-13 13:06   ` Sergey Bronnikov
  0 siblings, 0 replies; 5+ messages in thread
From: Sergey Bronnikov @ 2020-04-13 13:06 UTC (permalink / raw)
  To: Serge Petrenko; +Cc: o.piskunov, tml

Hi!

Sergey, thanks for review.

On 15:55 Sat 11 Apr , Serge Petrenko wrote:
> Hi! Thanks for the patch!
>
> I think the check can’t fail after `box.snapshot()` even without wait_cond().
> You’ve already waited for the index to build right before `box.snapshot()`.
> It’ s better to leave the last 2 checks without wait_cond to emphasize that
> nothing could happen during `box.snapshot()`.

You are wrong. Test is failed when wait_cond() below 'box.snapshot()' removed.

S.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Tarantool-patches] [PATCH] Fix flaky test engine/ddl
  2020-04-10 13:27 [Tarantool-patches] [PATCH] Fix flaky test engine/ddl Sergey Bronnikov
  2020-04-11 12:55 ` Serge Petrenko
  2020-04-13  8:45 ` Oleg Piskunov
@ 2020-04-15 11:14 ` Kirill Yukhin
  2 siblings, 0 replies; 5+ messages in thread
From: Kirill Yukhin @ 2020-04-15 11:14 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: o.piskunov, tarantool-patches

Hello,


On 10 апр 16:27, Sergey Bronnikov wrote:
> Test was a flaky from the beginning 39d0e4273dde2dbb3e46aea35310379e98e7cc64
> Time of building indexes varies from time to time and the problem was due to
> abcense of synchronization in index building and checking numbers of these
> indexes.
> 
> Fixes #4353
> ---
>  test/engine/ddl.result   | 15 +++++++++------
>  test/engine/ddl.test.lua | 14 ++++++++------
>  test/engine/suite.ini    |  3 +--
>  3 files changed, 18 insertions(+), 14 deletions(-)

I've checked your patch into 2.2, 2.3 and master.

--
Regards, Kirill Yukhin

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-04-15 11:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-10 13:27 [Tarantool-patches] [PATCH] Fix flaky test engine/ddl Sergey Bronnikov
2020-04-11 12:55 ` Serge Petrenko
2020-04-13 13:06   ` Sergey Bronnikov
2020-04-13  8:45 ` Oleg Piskunov
2020-04-15 11:14 ` Kirill Yukhin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox