Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Sergei Voronezhskii <sergw@tarantool.org>,
	Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: tarantool-patches@freelists.org
Subject: Re: [PATCH] test: enable parallel mode for wal_off tests
Date: Sat, 22 Sep 2018 04:44:17 +0300	[thread overview]
Message-ID: <20180922014417.zlp7hzjb6wzryx4q@tkn_work_nb> (raw)
In-Reply-To: <20180921124442.47554-1-sergw@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
> 

  reply	other threads:[~2018-09-22  1:44 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180922014417.zlp7hzjb6wzryx4q@tkn_work_nb \
    --to=alexander.turenko@tarantool.org \
    --cc=sergw@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH] test: enable parallel mode for wal_off tests' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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