Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH] test: enable parallel mode for wall_off tests
@ 2018-09-17 13:11 Sergei Voronezhskii
  2018-09-17 13:31 ` [tarantool-patches] " Alexander Turenko
  2018-11-28 14:11 ` [tarantool-patches] Re: [PATCH] " Kirill Yukhin
  0 siblings, 2 replies; 13+ messages in thread
From: Sergei Voronezhskii @ 2018-09-17 13:11 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Alexander Turenko

Use the proper way to cleanup tests.

Part of #2436
---
 test/wal_off/oom.result     | 4 ++--
 test/wal_off/oom.test.lua   | 4 ++--
 test/wal_off/suite.ini      | 2 +-
 test/wal_off/tuple.result   | 2 +-
 test/wal_off/tuple.test.lua | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/test/wal_off/oom.result b/test/wal_off/oom.result
index c47d16c46..65bb8c670 100644
--- a/test/wal_off/oom.result
+++ b/test/wal_off/oom.result
@@ -4,7 +4,7 @@ env = require('test_run')
 test_run = env.new()
 ---
 ...
-test_run:cmd('restart server default')
+test_run:cmd('restart server default with cleanup=1')
 test_run:cmd("push filter 'error: Failed to allocate [0-9]+ ' to 'error: Failed to allocate <NUM> '")
 ---
 - true
@@ -231,7 +231,7 @@ t = nil
 ---
 ...
 -- https://github.com/tarantool/tarantool/issues/962 index:delete() failed
-test_run:cmd('restart server default')
+test_run:cmd('restart server default with cleanup=1')
 arena_bytes = box.cfg.memtx_memory
 ---
 ...
diff --git a/test/wal_off/oom.test.lua b/test/wal_off/oom.test.lua
index 5c0ab8e73..89dba3f06 100644
--- a/test/wal_off/oom.test.lua
+++ b/test/wal_off/oom.test.lua
@@ -1,6 +1,6 @@
 env = require('test_run')
 test_run = env.new()
-test_run:cmd('restart server default')
+test_run:cmd('restart server default with cleanup=1')
 test_run:cmd("push filter 'error: Failed to allocate [0-9]+ ' to 'error: Failed to allocate <NUM> '")
 
 space = box.schema.space.create('tweedledum')
@@ -80,7 +80,7 @@ space:drop()
 t = nil
 
 -- https://github.com/tarantool/tarantool/issues/962 index:delete() failed
-test_run:cmd('restart server default')
+test_run:cmd('restart server default with cleanup=1')
 arena_bytes = box.cfg.memtx_memory
 str = string.rep('a', 15000) -- about size of index memory block
 
diff --git a/test/wal_off/suite.ini b/test/wal_off/suite.ini
index ad19eab10..cbb7cb341 100644
--- a/test/wal_off/suite.ini
+++ b/test/wal_off/suite.ini
@@ -2,4 +2,4 @@
 core = tarantool
 script = wal.lua
 description = tarantool/box, wal_mode = none
-is_parallel = False
+is_parallel = True
diff --git a/test/wal_off/tuple.result b/test/wal_off/tuple.result
index fa431e203..dcbd24daf 100644
--- a/test/wal_off/tuple.result
+++ b/test/wal_off/tuple.result
@@ -4,7 +4,7 @@ env = require('test_run')
 test_run = env.new()
 ---
 ...
-test_run:cmd("restart server default")
+test_run:cmd("restart server default with cleanup=1")
 -- 
 -- Test various tuple bugs which do not require a write ahead log.
 -- 
diff --git a/test/wal_off/tuple.test.lua b/test/wal_off/tuple.test.lua
index 19415a92d..859438f71 100644
--- a/test/wal_off/tuple.test.lua
+++ b/test/wal_off/tuple.test.lua
@@ -1,6 +1,6 @@
 env = require('test_run')
 test_run = env.new()
-test_run:cmd("restart server default")
+test_run:cmd("restart server default with cleanup=1")
 -- 
 -- Test various tuple bugs which do not require a write ahead log.
 -- 
-- 
2.18.0

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

* [tarantool-patches] Re: [PATCH] test: enable parallel mode for wall_off tests
  2018-09-17 13:11 [tarantool-patches] [PATCH] test: enable parallel mode for wall_off tests Sergei Voronezhskii
@ 2018-09-17 13:31 ` Alexander Turenko
  2018-09-18 16:10   ` [tarantool-patches] [PATCH v2] " Sergei Voronezhskii
  2018-11-28 14:11 ` [tarantool-patches] Re: [PATCH] " Kirill Yukhin
  1 sibling, 1 reply; 13+ messages in thread
From: Alexander Turenko @ 2018-09-17 13:31 UTC (permalink / raw)
  To: Sergei Voronezhskii; +Cc: tarantool-patches

Adding cleanup option is okay for me, but parallel run still fails and
it is not the time to enable it. I got two failures:

$ cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_BACKTRACE=ON && make -j
$ TEST_RUN_TESTS=$(for i in $(seq 1 100); do echo -n 'wal_off/ '; done) make test

[002] wal_off/lua.test.lua                                            [ fail ]
[002] 
[002] Test failed! Result content mismatch:
[002] --- wal_off/lua.result	Fri Apr 27 14:25:39 2018
[002] +++ wal_off/lua.reject	Mon Sep 17 16:25:40 2018
[002] @@ -91,7 +91,7 @@
[002]  ...
[002]  mktuple(100000)
[002]  ---
[002] -- count 100000 len 368553
[002] +- error: Failed to allocate 368569 bytes in slab allocator for memtx_tuple
[002]  ...
[002]  space:drop()
[002]  ---
[002] 

# reproduce file: /home/alex/projects/tarantool-meta/review/tarantool/test/var/reproduce/002_wal_off.list.yaml
---
- [wal_off/tuple.test.lua, null]
- [wal_off/iterator_lt_gt.test.lua, null]
- [wal_off/snapshot_stress.test.lua, null]
- [wal_off/rtree_benchmark.test.lua, null]
- [wal_off/lua.test.lua, null]

[008] wal_off/snapshot_stress.test.lua                                [ fail ]
[008] 
[008] Test failed! Result content mismatch:
[008] --- wal_off/snapshot_stress.result	Fri Apr 27 14:25:39 2018
[008] +++ wal_off/snapshot_stress.reject	Mon Sep 17 16:25:41 2018
[008] @@ -375,7 +375,7 @@
[008]  ...
[008]  snaps_find_status;
[008]  ---
[008] -- snaps found
[008] +- where are my snapshots?
[008]  ...
[008]  snapshot_check_failed = false
[008]  while #snaps > initial_snap_count do
[008] 

# reproduce file: /home/alex/projects/tarantool-meta/review/tarantool/test/var/reproduce/008_wal_off.list.yaml
---
- [wal_off/lua.test.lua, null]
- [wal_off/expirationd.test.lua, null]
- [wal_off/expirationd.test.lua, null]
- [wal_off/wal_mode.test.lua, null]
- [wal_off/func_max.test.lua, null]
- [wal_off/expirationd.test.lua, null]
- [wal_off/snapshot_stress.test.lua, null]

Please, reproduce and elaborate.

BTW, you are forgot the branch name: sergw/enable-parallel-test-wal-off.

WBR, Alexander Turenko.

On Mon, Sep 17, 2018 at 04:11:55PM +0300, Sergei Voronezhskii wrote:
> Use the proper way to cleanup tests.
> 
> Part of #2436
> ---
>  test/wal_off/oom.result     | 4 ++--
>  test/wal_off/oom.test.lua   | 4 ++--
>  test/wal_off/suite.ini      | 2 +-
>  test/wal_off/tuple.result   | 2 +-
>  test/wal_off/tuple.test.lua | 2 +-
>  5 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/test/wal_off/oom.result b/test/wal_off/oom.result
> index c47d16c46..65bb8c670 100644
> --- a/test/wal_off/oom.result
> +++ b/test/wal_off/oom.result
> @@ -4,7 +4,7 @@ env = require('test_run')
>  test_run = env.new()
>  ---
>  ...
> -test_run:cmd('restart server default')
> +test_run:cmd('restart server default with cleanup=1')
>  test_run:cmd("push filter 'error: Failed to allocate [0-9]+ ' to 'error: Failed to allocate <NUM> '")
>  ---
>  - true
> @@ -231,7 +231,7 @@ t = nil
>  ---
>  ...
>  -- https://github.com/tarantool/tarantool/issues/962 index:delete() failed
> -test_run:cmd('restart server default')
> +test_run:cmd('restart server default with cleanup=1')
>  arena_bytes = box.cfg.memtx_memory
>  ---
>  ...
> diff --git a/test/wal_off/oom.test.lua b/test/wal_off/oom.test.lua
> index 5c0ab8e73..89dba3f06 100644
> --- a/test/wal_off/oom.test.lua
> +++ b/test/wal_off/oom.test.lua
> @@ -1,6 +1,6 @@
>  env = require('test_run')
>  test_run = env.new()
> -test_run:cmd('restart server default')
> +test_run:cmd('restart server default with cleanup=1')
>  test_run:cmd("push filter 'error: Failed to allocate [0-9]+ ' to 'error: Failed to allocate <NUM> '")
>  
>  space = box.schema.space.create('tweedledum')
> @@ -80,7 +80,7 @@ space:drop()
>  t = nil
>  
>  -- https://github.com/tarantool/tarantool/issues/962 index:delete() failed
> -test_run:cmd('restart server default')
> +test_run:cmd('restart server default with cleanup=1')
>  arena_bytes = box.cfg.memtx_memory
>  str = string.rep('a', 15000) -- about size of index memory block
>  
> diff --git a/test/wal_off/suite.ini b/test/wal_off/suite.ini
> index ad19eab10..cbb7cb341 100644
> --- a/test/wal_off/suite.ini
> +++ b/test/wal_off/suite.ini
> @@ -2,4 +2,4 @@
>  core = tarantool
>  script = wal.lua
>  description = tarantool/box, wal_mode = none
> -is_parallel = False
> +is_parallel = True
> diff --git a/test/wal_off/tuple.result b/test/wal_off/tuple.result
> index fa431e203..dcbd24daf 100644
> --- a/test/wal_off/tuple.result
> +++ b/test/wal_off/tuple.result
> @@ -4,7 +4,7 @@ env = require('test_run')
>  test_run = env.new()
>  ---
>  ...
> -test_run:cmd("restart server default")
> +test_run:cmd("restart server default with cleanup=1")
>  -- 
>  -- Test various tuple bugs which do not require a write ahead log.
>  -- 
> diff --git a/test/wal_off/tuple.test.lua b/test/wal_off/tuple.test.lua
> index 19415a92d..859438f71 100644
> --- a/test/wal_off/tuple.test.lua
> +++ b/test/wal_off/tuple.test.lua
> @@ -1,6 +1,6 @@
>  env = require('test_run')
>  test_run = env.new()
> -test_run:cmd("restart server default")
> +test_run:cmd("restart server default with cleanup=1")
>  -- 
>  -- Test various tuple bugs which do not require a write ahead log.
>  -- 
> -- 
> 2.18.0
> 

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

* [tarantool-patches] [PATCH v2] test: enable parallel mode for wall_off tests
  2018-09-17 13:31 ` [tarantool-patches] " Alexander Turenko
@ 2018-09-18 16:10   ` Sergei Voronezhskii
  2018-09-19 16:57     ` [tarantool-patches] " Alexander Turenko
  0 siblings, 1 reply; 13+ messages in thread
From: Sergei Voronezhskii @ 2018-09-18 16:10 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Alexander Turenko

Use the proper way to cleanup tests.

Part of #2436
---
 test/wal_off/lua.result               | 1 +
 test/wal_off/lua.test.lua             | 1 +
 test/wal_off/oom.result               | 4 ++--
 test/wal_off/oom.test.lua             | 4 ++--
 test/wal_off/snapshot_stress.result   | 1 +
 test/wal_off/snapshot_stress.test.lua | 1 +
 test/wal_off/suite.ini                | 2 +-
 test/wal_off/tuple.result             | 2 +-
 test/wal_off/tuple.test.lua           | 2 +-
 9 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/test/wal_off/lua.result b/test/wal_off/lua.result
index f075feb24..738cad27f 100644
--- a/test/wal_off/lua.result
+++ b/test/wal_off/lua.result
@@ -4,6 +4,7 @@ env = require('test_run')
 test_run = env.new()
 ---
 ...
+test_run:cmd("restart server default with cleanup=1")
 space = box.schema.space.create('tweedledum')
 ---
 ...
diff --git a/test/wal_off/lua.test.lua b/test/wal_off/lua.test.lua
index 7daf9f3f6..02ec9e795 100644
--- a/test/wal_off/lua.test.lua
+++ b/test/wal_off/lua.test.lua
@@ -1,5 +1,6 @@
 env = require('test_run')
 test_run = env.new()
+test_run:cmd("restart server default with cleanup=1")
 space = box.schema.space.create('tweedledum')
 index1 = space:create_index('primary', { type ='hash', parts = {1, 'string'}, unique = true })
 index2 = space:create_index('secondary', { type = 'tree', parts = {2, 'unsigned'}, unique = false })
diff --git a/test/wal_off/oom.result b/test/wal_off/oom.result
index c47d16c46..65bb8c670 100644
--- a/test/wal_off/oom.result
+++ b/test/wal_off/oom.result
@@ -4,7 +4,7 @@ env = require('test_run')
 test_run = env.new()
 ---
 ...
-test_run:cmd('restart server default')
+test_run:cmd('restart server default with cleanup=1')
 test_run:cmd("push filter 'error: Failed to allocate [0-9]+ ' to 'error: Failed to allocate <NUM> '")
 ---
 - true
@@ -231,7 +231,7 @@ t = nil
 ---
 ...
 -- https://github.com/tarantool/tarantool/issues/962 index:delete() failed
-test_run:cmd('restart server default')
+test_run:cmd('restart server default with cleanup=1')
 arena_bytes = box.cfg.memtx_memory
 ---
 ...
diff --git a/test/wal_off/oom.test.lua b/test/wal_off/oom.test.lua
index 5c0ab8e73..89dba3f06 100644
--- a/test/wal_off/oom.test.lua
+++ b/test/wal_off/oom.test.lua
@@ -1,6 +1,6 @@
 env = require('test_run')
 test_run = env.new()
-test_run:cmd('restart server default')
+test_run:cmd('restart server default with cleanup=1')
 test_run:cmd("push filter 'error: Failed to allocate [0-9]+ ' to 'error: Failed to allocate <NUM> '")
 
 space = box.schema.space.create('tweedledum')
@@ -80,7 +80,7 @@ space:drop()
 t = nil
 
 -- https://github.com/tarantool/tarantool/issues/962 index:delete() failed
-test_run:cmd('restart server default')
+test_run:cmd('restart server default with cleanup=1')
 arena_bytes = box.cfg.memtx_memory
 str = string.rep('a', 15000) -- about size of index memory block
 
diff --git a/test/wal_off/snapshot_stress.result b/test/wal_off/snapshot_stress.result
index 29cea8c04..22e97c6c8 100644
--- a/test/wal_off/snapshot_stress.result
+++ b/test/wal_off/snapshot_stress.result
@@ -8,6 +8,7 @@ env = require('test_run')
 test_run = env.new()
 ---
 ...
+test_run:cmd("restart server default with cleanup=1")
 -- Settings: You may increase theese value to make test longer
 -- number of worker fibers:
 workers_count = 80
diff --git a/test/wal_off/snapshot_stress.test.lua b/test/wal_off/snapshot_stress.test.lua
index 34b3e7b78..990d5e63b 100644
--- a/test/wal_off/snapshot_stress.test.lua
+++ b/test/wal_off/snapshot_stress.test.lua
@@ -4,6 +4,7 @@
 -- Snapshots are made every snapshot_interval seconds and then checked for consistency
 env = require('test_run')
 test_run = env.new()
+test_run:cmd("restart server default with cleanup=1")
 -- Settings: You may increase theese value to make test longer
 -- number of worker fibers:
 workers_count = 80
diff --git a/test/wal_off/suite.ini b/test/wal_off/suite.ini
index ad19eab10..cbb7cb341 100644
--- a/test/wal_off/suite.ini
+++ b/test/wal_off/suite.ini
@@ -2,4 +2,4 @@
 core = tarantool
 script = wal.lua
 description = tarantool/box, wal_mode = none
-is_parallel = False
+is_parallel = True
diff --git a/test/wal_off/tuple.result b/test/wal_off/tuple.result
index fa431e203..dcbd24daf 100644
--- a/test/wal_off/tuple.result
+++ b/test/wal_off/tuple.result
@@ -4,7 +4,7 @@ env = require('test_run')
 test_run = env.new()
 ---
 ...
-test_run:cmd("restart server default")
+test_run:cmd("restart server default with cleanup=1")
 -- 
 -- Test various tuple bugs which do not require a write ahead log.
 -- 
diff --git a/test/wal_off/tuple.test.lua b/test/wal_off/tuple.test.lua
index 19415a92d..859438f71 100644
--- a/test/wal_off/tuple.test.lua
+++ b/test/wal_off/tuple.test.lua
@@ -1,6 +1,6 @@
 env = require('test_run')
 test_run = env.new()
-test_run:cmd("restart server default")
+test_run:cmd("restart server default with cleanup=1")
 -- 
 -- Test various tuple bugs which do not require a write ahead log.
 -- 
-- 
2.18.0

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

* [tarantool-patches] Re: [PATCH v2] test: enable parallel mode for wall_off tests
  2018-09-18 16:10   ` [tarantool-patches] [PATCH v2] " Sergei Voronezhskii
@ 2018-09-19 16:57     ` Alexander Turenko
  2018-09-21 12:44       ` [tarantool-patches] [PATCH] test: enable parallel mode for wal_off tests Sergei Voronezhskii
  2018-11-27 13:09       ` Re[2]: [PATCH v2] test: enable parallel mode for wall_off tests Sergei Voronezhskii
  0 siblings, 2 replies; 13+ messages in thread
From: Alexander Turenko @ 2018-09-19 16:57 UTC (permalink / raw)
  To: Sergei Voronezhskii; +Cc: tarantool-patches

Hi!

Comments are below.

WBR, Alexander Turenko.

Maybe we should place the fix on top of 1.9. I asked Kostya O. and
Kirill Yu. about that in the chat, please ping them and proceed
appropriately.

> enable parallel mode for wall_off tests

wall -> wal

> diff --git a/test/wal_off/lua.test.lua b/test/wal_off/lua.test.lua
> index 7daf9f3f6..02ec9e795 100644
> --- a/test/wal_off/lua.test.lua
> +++ b/test/wal_off/lua.test.lua
> @@ -1,5 +1,6 @@
>  env = require('test_run')
>  test_run = env.new()
> +test_run:cmd("restart server default with cleanup=1")

So the approach is to restart the server with clean up for each tests.
It will slows down the execution. The another way you propose (w/ spaces
renaming) seems to be better for me.

Anyway, you don't describe a reason why exactly this is needed. How we
can meet an inconsistent state if a server was not restarted or
restarted with clean up? I propose to describe problems you fix in the
commit message, otherwise I'll need to investigate the issue again after
you to understand it and give the review.

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

* [tarantool-patches] [PATCH] test: enable parallel mode for wal_off tests
  2018-09-19 16:57     ` [tarantool-patches] " Alexander Turenko
@ 2018-09-21 12:44       ` Sergei Voronezhskii
  2018-09-22  1:44         ` Alexander Turenko
  2018-11-27 13:09       ` Re[2]: [PATCH v2] test: enable parallel mode for wall_off tests Sergei Voronezhskii
  1 sibling, 1 reply; 13+ messages in thread
From: Sergei Voronezhskii @ 2018-09-21 12:44 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Alexander Turenko

Use the proper way to cleanup tests.
- tuple + lua needs more defragmented memory
- snapshot_stress checks for count of checkpoints
- need to cleanup default because of some tests dont drop spaces

Part of #2436
---

BRANCH: sergw/enable-parallel-test-wal-off-clean

 test/wal_off/oom.result     | 2 +-
 test/wal_off/oom.test.lua   | 2 +-
 test/wal_off/suite.ini      | 2 +-
 test/wal_off/tuple.result   | 2 +-
 test/wal_off/tuple.test.lua | 2 +-
 test/wal_off/wal.lua        | 5 +++--
 6 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/test/wal_off/oom.result b/test/wal_off/oom.result
index c47d16c46..90bc53d22 100644
--- a/test/wal_off/oom.result
+++ b/test/wal_off/oom.result
@@ -4,7 +4,7 @@ env = require('test_run')
 test_run = env.new()
 ---
 ...
-test_run:cmd('restart server default')
+test_run:cmd('restart server default with cleanup=1')
 test_run:cmd("push filter 'error: Failed to allocate [0-9]+ ' to 'error: Failed to allocate <NUM> '")
 ---
 - true
diff --git a/test/wal_off/oom.test.lua b/test/wal_off/oom.test.lua
index 5c0ab8e73..8e6e14046 100644
--- a/test/wal_off/oom.test.lua
+++ b/test/wal_off/oom.test.lua
@@ -1,6 +1,6 @@
 env = require('test_run')
 test_run = env.new()
-test_run:cmd('restart server default')
+test_run:cmd('restart server default with cleanup=1')
 test_run:cmd("push filter 'error: Failed to allocate [0-9]+ ' to 'error: Failed to allocate <NUM> '")
 
 space = box.schema.space.create('tweedledum')
diff --git a/test/wal_off/suite.ini b/test/wal_off/suite.ini
index ad19eab10..cbb7cb341 100644
--- a/test/wal_off/suite.ini
+++ b/test/wal_off/suite.ini
@@ -2,4 +2,4 @@
 core = tarantool
 script = wal.lua
 description = tarantool/box, wal_mode = none
-is_parallel = False
+is_parallel = True
diff --git a/test/wal_off/tuple.result b/test/wal_off/tuple.result
index fa431e203..6ea3814fc 100644
--- a/test/wal_off/tuple.result
+++ b/test/wal_off/tuple.result
@@ -4,7 +4,7 @@ env = require('test_run')
 test_run = env.new()
 ---
 ...
-test_run:cmd("restart server default")
+test_run:cmd('restart server default with cleanup=1')
 -- 
 -- Test various tuple bugs which do not require a write ahead log.
 -- 
diff --git a/test/wal_off/tuple.test.lua b/test/wal_off/tuple.test.lua
index 19415a92d..6962f35ad 100644
--- a/test/wal_off/tuple.test.lua
+++ b/test/wal_off/tuple.test.lua
@@ -1,6 +1,6 @@
 env = require('test_run')
 test_run = env.new()
-test_run:cmd("restart server default")
+test_run:cmd('restart server default with cleanup=1')
 -- 
 -- Test various tuple bugs which do not require a write ahead log.
 -- 
diff --git a/test/wal_off/wal.lua b/test/wal_off/wal.lua
index 6e4afbe4d..4005e78cd 100644
--- a/test/wal_off/wal.lua
+++ b/test/wal_off/wal.lua
@@ -2,9 +2,10 @@
 
 box.cfg{
     listen              = os.getenv("LISTEN"),
-    memtx_memory        = 107374182,
+    memtx_memory        = 157374182,
     pid_file            = "tarantool.pid",
-    wal_mode            = "none"
+    wal_mode            = "none",
+    checkpoint_count    = 100
 }
 
 require('console').listen(os.getenv('ADMIN'))
-- 
2.18.0

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

* Re: [PATCH] test: enable parallel mode for wal_off tests
  2018-09-21 12:44       ` [tarantool-patches] [PATCH] test: enable parallel mode for wal_off tests Sergei Voronezhskii
@ 2018-09-22  1:44         ` Alexander Turenko
  2018-09-23 22:40           ` Re[2]: " Sergei Voronezhskii
  2018-09-25 17:10           ` Vladimir Davydov
  0 siblings, 2 replies; 13+ messages in thread
From: Alexander Turenko @ 2018-09-22  1:44 UTC (permalink / raw)
  To: Sergei Voronezhskii, Vladimir Davydov; +Cc: tarantool-patches

Hi!

Thanks for the investigation.

The diff looks good for me and works for me. I have minor comments for
the commit message wording. I'll leave my comments below and the patch
is not needed to re-review with me: please, proceed with Vova and
Kirill.

Vova, can you please confirm that memtx_memory option should be
increased in the test? I mean confirm that it is the test problem and is
not a problem of tarantool itself.

WBR, Alexander Turenko.

On Fri, Sep 21, 2018 at 03:44:42PM +0300, Sergei Voronezhskii wrote:
> Use the proper way to cleanup tests.

'Use the proper way to cleanup tests' is too common and does not give a
reader useful information. It also cannot be a title of the list below,
because not all items below are about clean up. Proposed to remove the
sentence.

> - tuple + lua needs more defragmented memory

Proposed: memtx_memory is increased, because the test '{TEST NAME HERE}'
after '{TEST NAME HERE}' failed with the error '{ERROR HERE}' despite
collectgarbage('collect') calls after cases with huge/many tuples. The
statistics before the allocation fail gives the following values:
{VALUES HERE}. The reason of the fail seems to be a slab memory
fragmentation. It is not clear for now whether we should consider this
as a tarantool issue.

> - snapshot_stress checks for count of checkpoints

Proposed: 'snapshot_stress' test counts snapshot files present in the
working directory and can reach the default 'checkpoint_count' value (2)
if a previous test write its snapshots before.

> - need to cleanup default because of some tests dont drop spaces
> 

Proposed: restarting the default server w/o cleaning a working directory
can leave a snapshot that holds a state saved at the middle of a test,
before dropping of the space 'tweedledum' (because WAL is disabled),
that can cause the error '{ERROR HERE}' for a following test.

Sorry for nitpicking, that was to have a clear problem statements and
don't reinvestigate it again in the future (if something like occurs).

> Part of #2436
> ---
> 
> BRANCH: sergw/enable-parallel-test-wal-off-clean
> 
>  test/wal_off/oom.result     | 2 +-
>  test/wal_off/oom.test.lua   | 2 +-
>  test/wal_off/suite.ini      | 2 +-
>  test/wal_off/tuple.result   | 2 +-
>  test/wal_off/tuple.test.lua | 2 +-
>  test/wal_off/wal.lua        | 5 +++--
>  6 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/test/wal_off/oom.result b/test/wal_off/oom.result
> index c47d16c46..90bc53d22 100644
> --- a/test/wal_off/oom.result
> +++ b/test/wal_off/oom.result
> @@ -4,7 +4,7 @@ env = require('test_run')
>  test_run = env.new()
>  ---
>  ...
> -test_run:cmd('restart server default')
> +test_run:cmd('restart server default with cleanup=1')
>  test_run:cmd("push filter 'error: Failed to allocate [0-9]+ ' to 'error: Failed to allocate <NUM> '")
>  ---
>  - true
> diff --git a/test/wal_off/oom.test.lua b/test/wal_off/oom.test.lua
> index 5c0ab8e73..8e6e14046 100644
> --- a/test/wal_off/oom.test.lua
> +++ b/test/wal_off/oom.test.lua
> @@ -1,6 +1,6 @@
>  env = require('test_run')
>  test_run = env.new()
> -test_run:cmd('restart server default')
> +test_run:cmd('restart server default with cleanup=1')
>  test_run:cmd("push filter 'error: Failed to allocate [0-9]+ ' to 'error: Failed to allocate <NUM> '")
>  
>  space = box.schema.space.create('tweedledum')
> diff --git a/test/wal_off/suite.ini b/test/wal_off/suite.ini
> index ad19eab10..cbb7cb341 100644
> --- a/test/wal_off/suite.ini
> +++ b/test/wal_off/suite.ini
> @@ -2,4 +2,4 @@
>  core = tarantool
>  script = wal.lua
>  description = tarantool/box, wal_mode = none
> -is_parallel = False
> +is_parallel = True
> diff --git a/test/wal_off/tuple.result b/test/wal_off/tuple.result
> index fa431e203..6ea3814fc 100644
> --- a/test/wal_off/tuple.result
> +++ b/test/wal_off/tuple.result
> @@ -4,7 +4,7 @@ env = require('test_run')
>  test_run = env.new()
>  ---
>  ...
> -test_run:cmd("restart server default")
> +test_run:cmd('restart server default with cleanup=1')
>  -- 
>  -- Test various tuple bugs which do not require a write ahead log.
>  -- 
> diff --git a/test/wal_off/tuple.test.lua b/test/wal_off/tuple.test.lua
> index 19415a92d..6962f35ad 100644
> --- a/test/wal_off/tuple.test.lua
> +++ b/test/wal_off/tuple.test.lua
> @@ -1,6 +1,6 @@
>  env = require('test_run')
>  test_run = env.new()
> -test_run:cmd("restart server default")
> +test_run:cmd('restart server default with cleanup=1')
>  -- 
>  -- Test various tuple bugs which do not require a write ahead log.
>  -- 
> diff --git a/test/wal_off/wal.lua b/test/wal_off/wal.lua
> index 6e4afbe4d..4005e78cd 100644
> --- a/test/wal_off/wal.lua
> +++ b/test/wal_off/wal.lua
> @@ -2,9 +2,10 @@
>  
>  box.cfg{
>      listen              = os.getenv("LISTEN"),
> -    memtx_memory        = 107374182,
> +    memtx_memory        = 157374182,
>      pid_file            = "tarantool.pid",
> -    wal_mode            = "none"
> +    wal_mode            = "none",
> +    checkpoint_count    = 100
>  }
>  
>  require('console').listen(os.getenv('ADMIN'))
> -- 
> 2.18.0
> 

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

* Re[2]: [PATCH] test: enable parallel mode for wal_off tests
  2018-09-22  1:44         ` Alexander Turenko
@ 2018-09-23 22:40           ` Sergei Voronezhskii
  2018-09-23 22:53             ` [tarantool-patches] " Alexander Turenko
  2018-09-25 17:10           ` Vladimir Davydov
  1 sibling, 1 reply; 13+ messages in thread
From: Sergei Voronezhskii @ 2018-09-23 22:40 UTC (permalink / raw)
  To: Alexander Turenko; +Cc: tarantool-patches, Vladimir Davydov

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

Hi!

I've amended the commit message.
BRANCH: https://github.com/tarantool/tarantool/tree/sergw/enable-parallel-test-wal-off-clean


>Суббота, 22 сентября 2018, 4:44 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
>
>Hi!
>
>Thanks for the investigation.
>
>The diff looks good for me and works for me. I have minor comments for
>the commit message wording. I'll leave my comments below and the patch
>is not needed to re-review with me: please, proceed with Vova and
>Kirill.
>
>Vova, can you please confirm that memtx_memory option should be
>increased in the test? I mean confirm that it is the test problem and is
>not a problem of tarantool itself.
>
>WBR, Alexander Turenko.
>
>On Fri, Sep 21, 2018 at 03:44:42PM +0300, Sergei Voronezhskii wrote:
>> Use the proper way to cleanup tests.
>
>'Use the proper way to cleanup tests' is too common and does not give a
>reader useful information. It also cannot be a title of the list below,
>because not all items below are about clean up. Proposed to remove the
>sentence.
>
>> - tuple + lua needs more defragmented memory
>
>Proposed: memtx_memory is increased, because the test '{TEST NAME HERE}'
>after '{TEST NAME HERE}' failed with the error '{ERROR HERE}' despite
>collectgarbage('collect') calls after cases with huge/many tuples. The
>statistics before the allocation fail gives the following values:
>{VALUES HERE}. The reason of the fail seems to be a slab memory
>fragmentation. It is not clear for now whether we should consider this
>as a tarantool issue.
>
>> - snapshot_stress checks for count of checkpoints
>
>Proposed: 'snapshot_stress' test counts snapshot files present in the
>working directory and can reach the default 'checkpoint_count' value (2)
>if a previous test write its snapshots before.
>
>> - need to cleanup default because of some tests dont drop spaces
>> 
>
>Proposed: restarting the default server w/o cleaning a working directory
>can leave a snapshot that holds a state saved at the middle of a test,
>before dropping of the space 'tweedledum' (because WAL is disabled),
>that can cause the error '{ERROR HERE}' for a following test.
>
>Sorry for nitpicking, that was to have a clear problem statements and
>don't reinvestigate it again in the future (if something like occurs).
>
>> Part of #2436
>> ---
>> 
>> BRANCH: sergw/enable-parallel-test-wal-off-clean
>> 
>>  test/wal_off/oom.result     | 2 +-
>>  test/wal_off/oom.test.lua   | 2 +-
>>  test/wal_off/suite.ini      | 2 +-
>>  test/wal_off/tuple.result   | 2 +-
>>  test/wal_off/tuple.test.lua | 2 +-
>>  test/wal_off/wal.lua        | 5 +++--
>>  6 files changed, 8 insertions(+), 7 deletions(-)
>> 
>> diff --git a/test/wal_off/oom.result b/test/wal_off/oom.result
>> index c47d16c46..90bc53d22 100644
>> --- a/test/wal_off/oom.result
>> +++ b/test/wal_off/oom.result
>> @@ -4,7 +4,7 @@ env = require('test_run')
>>  test_run = env.new()
>>  ---
>>  ...
>> -test_run:cmd('restart server default')
>> +test_run:cmd('restart server default with cleanup=1')
>>  test_run:cmd("push filter 'error: Failed to allocate [0-9]+ ' to 'error: Failed to allocate <NUM> '")
>>  ---
>>  - true
>> diff --git a/test/wal_off/oom.test.lua b/test/wal_off/oom.test.lua
>> index 5c0ab8e73..8e6e14046 100644
>> --- a/test/wal_off/oom.test.lua
>> +++ b/test/wal_off/oom.test.lua
>> @@ -1,6 +1,6 @@
>>  env = require('test_run')
>>  test_run = env.new()
>> -test_run:cmd('restart server default')
>> +test_run:cmd('restart server default with cleanup=1')
>>  test_run:cmd("push filter 'error: Failed to allocate [0-9]+ ' to 'error: Failed to allocate <NUM> '")
>> 
>>  space = box.schema.space.create('tweedledum')
>> diff --git a/test/wal_off/suite.ini b/test/wal_off/suite.ini
>> index ad19eab10..cbb7cb341 100644
>> --- a/test/wal_off/suite.ini
>> +++ b/test/wal_off/suite.ini
>> @@ -2,4 +2,4 @@
>>  core = tarantool
>>  script = wal.lua
>>  description = tarantool/box, wal_mode = none
>> -is_parallel = False
>> +is_parallel = True
>> diff --git a/test/wal_off/tuple.result b/test/wal_off/tuple.result
>> index fa431e203..6ea3814fc 100644
>> --- a/test/wal_off/tuple.result
>> +++ b/test/wal_off/tuple.result
>> @@ -4,7 +4,7 @@ env = require('test_run')
>>  test_run = env.new()
>>  ---
>>  ...
>> -test_run:cmd("restart server default")
>> +test_run:cmd('restart server default with cleanup=1')
>>  -- 
>>  -- Test various tuple bugs which do not require a write ahead log.
>>  -- 
>> diff --git a/test/wal_off/tuple.test.lua b/test/wal_off/tuple.test.lua
>> index 19415a92d..6962f35ad 100644
>> --- a/test/wal_off/tuple.test.lua
>> +++ b/test/wal_off/tuple.test.lua
>> @@ -1,6 +1,6 @@
>>  env = require('test_run')
>>  test_run = env.new()
>> -test_run:cmd("restart server default")
>> +test_run:cmd('restart server default with cleanup=1')
>>  -- 
>>  -- Test various tuple bugs which do not require a write ahead log.
>>  -- 
>> diff --git a/test/wal_off/wal.lua b/test/wal_off/wal.lua
>> index 6e4afbe4d..4005e78cd 100644
>> --- a/test/wal_off/wal.lua
>> +++ b/test/wal_off/wal.lua
>> @@ -2,9 +2,10 @@
>> 
>>  box.cfg{
>>      listen              = os.getenv("LISTEN"),
>> -    memtx_memory        = 107374182,
>> +    memtx_memory        = 157374182,
>>      pid_file            = "tarantool.pid",
>> -    wal_mode            = "none"
>> +    wal_mode            = "none",
>> +    checkpoint_count    = 100
>>  }
>> 
>>  require('console').listen(os.getenv('ADMIN'))
>> -- 
>> 2.18.0
>> 


-- 
Sergei Voronezhskii

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

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

* [tarantool-patches] Re: [PATCH] test: enable parallel mode for wal_off tests
  2018-09-23 22:40           ` Re[2]: " Sergei Voronezhskii
@ 2018-09-23 22:53             ` Alexander Turenko
  0 siblings, 0 replies; 13+ messages in thread
From: Alexander Turenko @ 2018-09-23 22:53 UTC (permalink / raw)
  To: Sergei Voronezhskii; +Cc: tarantool-patches

Hi!

Thanks for updating. I briefly look into the new commit message.

> - need to cleanup default because of some tests dont drop spaces

Still there and looks like a part of the following sentence. Are you had
intention to remove it? If not, then, please, don't refer a default
server as just 'default', it makes the message unclear, and split the
sentences. If so, do it :)

> that can cause the error count of spaces for a following test.

I guess the error message sounds like "Space 'tweedledum' already
exists". Don't sure what 'the error count of spaces' means.

WBR, Alexander Turenko.

On Mon, Sep 24, 2018 at 01:40:58AM +0300, Sergei Voronezhskii wrote:
>    Hi!
>    I've amended the commit message.
>    BRANCH:
>    https://github.com/tarantool/tarantool/tree/sergw/enable-parallel-test-
>    wal-off-clean

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

* Re: [PATCH] test: enable parallel mode for wal_off tests
  2018-09-22  1:44         ` Alexander Turenko
  2018-09-23 22:40           ` Re[2]: " Sergei Voronezhskii
@ 2018-09-25 17:10           ` Vladimir Davydov
  2018-09-25 17:44             ` Alexander Turenko
  1 sibling, 1 reply; 13+ messages in thread
From: Vladimir Davydov @ 2018-09-25 17:10 UTC (permalink / raw)
  To: Alexander Turenko; +Cc: Sergei Voronezhskii, tarantool-patches

On Sat, Sep 22, 2018 at 04:44:17AM +0300, Alexander Turenko wrote:
> Vova, can you please confirm that memtx_memory option should be
> increased in the test? I mean confirm that it is the test problem and is
> not a problem of tarantool itself.

Looks like this is a bug in Tarantool after all:

  https://github.com/tarantool/tarantool/issues/3633#issuecomment-424425046

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

* Re: [PATCH] test: enable parallel mode for wal_off tests
  2018-09-25 17:10           ` Vladimir Davydov
@ 2018-09-25 17:44             ` Alexander Turenko
  0 siblings, 0 replies; 13+ messages in thread
From: Alexander Turenko @ 2018-09-25 17:44 UTC (permalink / raw)
  To: Vladimir Davydov; +Cc: Sergei Voronezhskii, tarantool-patches

On Tue, Sep 25, 2018 at 08:10:07PM +0300, Vladimir Davydov wrote:
> On Sat, Sep 22, 2018 at 04:44:17AM +0300, Alexander Turenko wrote:
> > Vova, can you please confirm that memtx_memory option should be
> > increased in the test? I mean confirm that it is the test problem and is
> > not a problem of tarantool itself.
> 
> Looks like this is a bug in Tarantool after all:
> 
>   https://github.com/tarantool/tarantool/issues/3633#issuecomment-424425046

Thanks!

I think the memtx_memory should be increased for this test suite anyway
and more straight test case should be added once the issue will be
fixed.

WBR, Alexander Turenko.

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

* Re[2]: [PATCH v2] test: enable parallel mode for wall_off tests
  2018-09-19 16:57     ` [tarantool-patches] " Alexander Turenko
  2018-09-21 12:44       ` [tarantool-patches] [PATCH] test: enable parallel mode for wal_off tests Sergei Voronezhskii
@ 2018-11-27 13:09       ` Sergei Voronezhskii
  1 sibling, 0 replies; 13+ messages in thread
From: Sergei Voronezhskii @ 2018-11-27 13:09 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Alexander Turenko, Kirill Yukhin, Vladimir Davydov

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

Hello

I've rebased on 2.1
BUILD:  https://travis-ci.org/tarantool/tarantool/builds/460238498
BRANCH:  https://github.com/tarantool/tarantool/tree/sergw/enable-parallel-test-wal-off-clean-2.1

>Среда, 19 сентября 2018, 19:57 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
>
>Hi!
>
>Comments are below.
>
>WBR, Alexander Turenko.
>
>Maybe we should place the fix on top of 1.9. I asked Kostya O. and
>Kirill Yu. about that in the chat, please ping them and proceed
>appropriately.
>
>> enable parallel mode for wall_off tests
>
>wall -> wal
>
>> diff --git a/test/wal_off/lua.test.lua b/test/wal_off/lua.test.lua
>> index 7daf9f3f6..02ec9e795 100644
>> --- a/test/wal_off/lua.test.lua
>> +++ b/test/wal_off/lua.test.lua
>> @@ -1,5 +1,6 @@
>>  env = require('test_run')
>>  test_run = env.new()
>> +test_run:cmd("restart server default with cleanup=1")
>
>So the approach is to restart the server with clean up for each tests.
>It will slows down the execution. The another way you propose (w/ spaces
>renaming) seems to be better for me.
>
>Anyway, you don't describe a reason why exactly this is needed. How we
>can meet an inconsistent state if a server was not restarted or
>restarted with clean up? I propose to describe problems you fix in the
>commit message, otherwise I'll need to investigate the issue again after
>you to understand it and give the review.


-- 
Sergei Voronezhskii

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

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

* [tarantool-patches] Re: [PATCH] test: enable parallel mode for wall_off tests
  2018-09-17 13:11 [tarantool-patches] [PATCH] test: enable parallel mode for wall_off tests Sergei Voronezhskii
  2018-09-17 13:31 ` [tarantool-patches] " Alexander Turenko
@ 2018-11-28 14:11 ` Kirill Yukhin
  1 sibling, 0 replies; 13+ messages in thread
From: Kirill Yukhin @ 2018-11-28 14:11 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Alexander Turenko

Hello,
On 17 Sep 16:11, Sergei Voronezhskii wrote:
> Use the proper way to cleanup tests.
> 
> Part of #2436
> ---
>  test/wal_off/oom.result     | 4 ++--
>  test/wal_off/oom.test.lua   | 4 ++--
>  test/wal_off/suite.ini      | 2 +-
>  test/wal_off/tuple.result   | 2 +-
>  test/wal_off/tuple.test.lua | 2 +-
>  5 files changed, 7 insertions(+), 7 deletions(-)

I've checked your patch into 2.1 branch.

--
Regards, Kirill Yukhin

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

* [tarantool-patches] Re: [PATCH] test: enable parallel mode for wall_off tests
  2018-09-13 16:30 [tarantool-patches] " Sergei Voronezhskii
@ 2018-09-15  1:13 ` Alexander Turenko
  0 siblings, 0 replies; 13+ messages in thread
From: Alexander Turenko @ 2018-09-15  1:13 UTC (permalink / raw)
  To: Sergei Voronezhskii; +Cc: tarantool-patches

Hi!

We look into the problem with Sergei together.

Found failing trace:

- [wal_off/wal_mode.test.lua, null]
- [wal_off/tuple.test.lua, null]
- [wal_off/expirationd.test.lua, null]

wal_mode.test.lua creates a snapshot (it contains space 'tweedledum'),
then tuple.test.lua restarts the default server, it bootstraps from the
snapshot (because WAL is off) and then expirationd.test.lua tries to
create the space again and fails.

The fix:

diff --git a/test/wal_off/tuple.test.lua b/test/wal_off/tuple.test.lua
index 19415a92d..859438f71 100644
--- a/test/wal_off/tuple.test.lua
+++ b/test/wal_off/tuple.test.lua
@@ -1,6 +1,6 @@
 env = require('test_run')
 test_run = env.new()
-test_run:cmd("restart server default")
+test_run:cmd("restart server default with cleanup=1")
 --
 -- Test various tuple bugs which do not require a write ahead log.
 --

WBR, Alexander Turenko.

On Thu, Sep 13, 2018 at 07:30:07PM +0300, Sergei Voronezhskii wrote:
> All tests in one suite must use different spaces, because this can
> cause tests to affect each other.
> 
> Part of #2436
> ---
>  test/wal_off/expirationd.result   | 2 +-
>  test/wal_off/expirationd.test.lua | 2 +-
>  test/wal_off/oom.result           | 2 +-
>  test/wal_off/oom.test.lua         | 2 +-
>  test/wal_off/suite.ini            | 2 +-
>  test/wal_off/wal_mode.result      | 2 +-
>  test/wal_off/wal_mode.test.lua    | 2 +-
>  7 files changed, 7 insertions(+), 7 deletions(-)
> 

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

end of thread, other threads:[~2018-11-28 14:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-17 13:11 [tarantool-patches] [PATCH] test: enable parallel mode for wall_off tests Sergei Voronezhskii
2018-09-17 13:31 ` [tarantool-patches] " Alexander Turenko
2018-09-18 16:10   ` [tarantool-patches] [PATCH v2] " Sergei Voronezhskii
2018-09-19 16:57     ` [tarantool-patches] " Alexander Turenko
2018-09-21 12:44       ` [tarantool-patches] [PATCH] test: enable parallel mode for wal_off tests Sergei Voronezhskii
2018-09-22  1:44         ` Alexander Turenko
2018-09-23 22:40           ` Re[2]: " Sergei Voronezhskii
2018-09-23 22:53             ` [tarantool-patches] " Alexander Turenko
2018-09-25 17:10           ` Vladimir Davydov
2018-09-25 17:44             ` Alexander Turenko
2018-11-27 13:09       ` Re[2]: [PATCH v2] test: enable parallel mode for wall_off tests Sergei Voronezhskii
2018-11-28 14:11 ` [tarantool-patches] Re: [PATCH] " Kirill Yukhin
  -- strict thread matches above, loose matches on Subject: below --
2018-09-13 16:30 [tarantool-patches] " Sergei Voronezhskii
2018-09-15  1:13 ` [tarantool-patches] " Alexander Turenko

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