[PATCH] test: rework box/misc test part regarding on_shutdown triggers

[PATCH] test: rework box/misc test part regarding on_shutdown triggers

[Serge Petrenko]

Rewrite the test so that it doesn't depend on timeouts, which can be
exceeded under high load, too much and simplify it a little.

Also extract the whole part regarding on_shutdown triggers into a
separate test, so that its sporadic failures would be easier to
investigate.

Part of #4134
---
https://github.com/tarantool/tarantool/issues/4134
https://github.com/tarantool/tarantool/tree/sp/gh-4134-fix-on-shutdown-tests

 test/box/misc.result                   | 161 -------------------------
 test/box/misc.test.lua                 |  62 ----------
 test/box/on_shutdown_triggers.result   | 160 ++++++++++++++++++++++++
 test/box/on_shutdown_triggers.test.lua |  62 ++++++++++
 4 files changed, 222 insertions(+), 223 deletions(-)
 create mode 100644 test/box/on_shutdown_triggers.result
 create mode 100644 test/box/on_shutdown_triggers.test.lua

diff --git a/test/box/misc.result b/test/box/misc.result
index e2b618c9c..a1f7a0990 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -1269,164 +1269,3 @@ box.cfg{too_long_threshold = too_long_threshold}
 s:drop()
 ---
 ...
---
--- gh-1607: on_shutdown triggers.
---
-f = function() print('on_shutdown 1') end
----
-...
-g = function() print('on_shutdown 2') end
----
-...
-h = function() print('on_shutdown 3') end
----
-...
--- Check that on_shutdown triggers may yield
--- and perform some complicated actions.
-fiber = require('fiber')
----
-...
-test_run:cmd("setopt delimiter ';'")
----
-- true
-...
-trig = function()
-    fiber.sleep(0.01)
-    fiber.yield()
-    box.schema.space.create("shutdown")
-    box.space.shutdown:create_index("pk")
-    box.space.shutdown:insert{1,2,3}
-    print('on_shutdown 4')
-end;
----
-...
-test_run:cmd("setopt delimiter ''");
----
-- true
-...
-_ = box.ctl.on_shutdown(f)
----
-...
-_ = box.ctl.on_shutdown(g)
----
-...
--- Check that replacing triggers works
-_ = box.ctl.on_shutdown(h, g)
----
-...
-_ = box.ctl.on_shutdown(trig)
----
-...
-test_run:cmd('restart server default')
-test_run:grep_log('default', 'on_shutdown 1', nil, {noreset=true})
----
-- on_shutdown 1
-...
-test_run:grep_log('default', 'on_shutdown 2', nil, {noreset=true})
----
-- null
-...
-test_run:grep_log('default', 'on_shutdown 3', nil, {noreset=true})
----
-- on_shutdown 3
-...
-test_run:grep_log('default', 'on_shutdown 4', nil, {noreset=true})
----
-- on_shutdown 4
-...
-box.space.shutdown:select{}
----
-- - [1, 2, 3]
-...
-box.space.shutdown:drop()
----
-...
--- Check that os.exit invokes triggers
-fiber = require("fiber")
----
-...
-test_run:cmd("create server test with script='box/proxy.lua'")
----
-- true
-...
-test_run:cmd("start server test")
----
-- true
-...
-logfile = test_run:eval("test", "box.cfg.log")[1]
----
-...
-test_run:cmd("stop server test")
----
-- true
-...
--- clean up any leftover logs
-require("fio").unlink(logfile)
----
-- true
-...
-test_run:cmd("start server test")
----
-- true
-...
-test_run:cmd("switch test")
----
-- true
-...
-_ = box.ctl.on_shutdown(function() print("on_shutdown 5") end)
----
-...
--- Check that we don't hang infinitely after os.exit()
--- even if the following code doesn't yield.
-fiber = require("fiber")
----
-...
-_ = fiber.create(function() fiber.sleep(0.05) os.exit() while true do end end)
----
-...
-test_run:cmd("switch default")
----
-- true
-...
-fiber.sleep(0.1)
----
-...
--- The server should be already stopped by os.exit(),
--- but start doesn't work without a prior call to stop.
-test_run:cmd("stop server test")
----
-- true
-...
-test_run:cmd("start server test")
----
-- true
-...
-test_run:cmd("switch test")
----
-- true
-...
-test_run:grep_log('test', 'on_shutdown 5', nil, {noreset=true})
----
-- on_shutdown 5
-...
--- make sure we exited because of os.exit(), not a signal.
-test_run:grep_log('test', 'signal', nil, {noreset=true})
----
-- null
-...
-test_run:cmd("switch default")
----
-- true
-...
-test_run:cmd("stop server test")
----
-- true
-...
-test_run:cmd("cleanup server test")
----
-- true
-...
-test_run:cmd("delete server test")
----
-- true
-...
diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
index 18128b299..cc6cb34fb 100644
--- a/test/box/misc.test.lua
+++ b/test/box/misc.test.lua
@@ -352,65 +352,3 @@ rows == expected_rows
 lsn == expected_lsn
 box.cfg{too_long_threshold = too_long_threshold}
 s:drop()
-
---
--- gh-1607: on_shutdown triggers.
---
-f = function() print('on_shutdown 1') end
-g = function() print('on_shutdown 2') end
-h = function() print('on_shutdown 3') end
--- Check that on_shutdown triggers may yield
--- and perform some complicated actions.
-fiber = require('fiber')
-test_run:cmd("setopt delimiter ';'")
-trig = function()
-    fiber.sleep(0.01)
-    fiber.yield()
-    box.schema.space.create("shutdown")
-    box.space.shutdown:create_index("pk")
-    box.space.shutdown:insert{1,2,3}
-    print('on_shutdown 4')
-end;
-test_run:cmd("setopt delimiter ''");
-_ = box.ctl.on_shutdown(f)
-_ = box.ctl.on_shutdown(g)
--- Check that replacing triggers works
-_ = box.ctl.on_shutdown(h, g)
-_ = box.ctl.on_shutdown(trig)
-test_run:cmd('restart server default')
-test_run:grep_log('default', 'on_shutdown 1', nil, {noreset=true})
-test_run:grep_log('default', 'on_shutdown 2', nil, {noreset=true})
-test_run:grep_log('default', 'on_shutdown 3', nil, {noreset=true})
-test_run:grep_log('default', 'on_shutdown 4', nil, {noreset=true})
-box.space.shutdown:select{}
-box.space.shutdown:drop()
-
--- Check that os.exit invokes triggers
-fiber = require("fiber")
-test_run:cmd("create server test with script='box/proxy.lua'")
-test_run:cmd("start server test")
-logfile = test_run:eval("test", "box.cfg.log")[1]
-test_run:cmd("stop server test")
--- clean up any leftover logs
-require("fio").unlink(logfile)
-test_run:cmd("start server test")
-test_run:cmd("switch test")
-_ = box.ctl.on_shutdown(function() print("on_shutdown 5") end)
--- Check that we don't hang infinitely after os.exit()
--- even if the following code doesn't yield.
-fiber = require("fiber")
-_ = fiber.create(function() fiber.sleep(0.05) os.exit() while true do end end)
-test_run:cmd("switch default")
-fiber.sleep(0.1)
--- The server should be already stopped by os.exit(),
--- but start doesn't work without a prior call to stop.
-test_run:cmd("stop server test")
-test_run:cmd("start server test")
-test_run:cmd("switch test")
-test_run:grep_log('test', 'on_shutdown 5', nil, {noreset=true})
--- make sure we exited because of os.exit(), not a signal.
-test_run:grep_log('test', 'signal', nil, {noreset=true})
-test_run:cmd("switch default")
-test_run:cmd("stop server test")
-test_run:cmd("cleanup server test")
-test_run:cmd("delete server test")
diff --git a/test/box/on_shutdown_triggers.result b/test/box/on_shutdown_triggers.result
new file mode 100644
index 000000000..da45f63a6
--- /dev/null
+++ b/test/box/on_shutdown_triggers.result
@@ -0,0 +1,160 @@
+env = require('test_run')
+---
+...
+test_run = env.new()
+---
+...
+--
+-- gh-1607: on_shutdown triggers.
+--
+f = function() print('on_shutdown 1') end
+---
+...
+g = function() print('on_shutdown 2') end
+---
+...
+h = function() print('on_shutdown 3') end
+---
+...
+-- Check that on_shutdown triggers may yield
+-- and perform some complicated actions.
+fiber = require('fiber')
+---
+...
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+trig = function()
+    fiber.sleep(0.01)
+    fiber.yield()
+    box.schema.space.create("shutdown")
+    box.space.shutdown:create_index("pk")
+    box.space.shutdown:insert{1,2,3}
+    print('on_shutdown 4')
+end;
+---
+...
+test_run:cmd("setopt delimiter ''");
+---
+- true
+...
+_ = box.ctl.on_shutdown(f)
+---
+...
+_ = box.ctl.on_shutdown(g)
+---
+...
+-- Check that replacing triggers works
+_ = box.ctl.on_shutdown(h, g)
+---
+...
+_ = box.ctl.on_shutdown(trig)
+---
+...
+test_run:cmd('restart server default')
+test_run:grep_log('default', 'on_shutdown 1', nil, {noreset=true})
+---
+- on_shutdown 1
+...
+test_run:grep_log('default', 'on_shutdown 2', nil, {noreset=true})
+---
+- null
+...
+test_run:grep_log('default', 'on_shutdown 3', nil, {noreset=true})
+---
+- on_shutdown 3
+...
+test_run:grep_log('default', 'on_shutdown 4', nil, {noreset=true})
+---
+- on_shutdown 4
+...
+box.space.shutdown:select{}
+---
+- - [1, 2, 3]
+...
+box.space.shutdown:drop()
+---
+...
+-- Check that os.exit invokes on_shutdown triggers
+fiber = require("fiber")
+---
+...
+test_run:cmd("create server test with script='box/proxy.lua'")
+---
+- true
+...
+test_run:cmd("start server test")
+---
+- true
+...
+logfile = test_run:eval("test", "box.cfg.log")[1]
+---
+...
+test_run:cmd("stop server test")
+---
+- true
+...
+-- clean up any leftover logs
+require("fio").unlink(logfile)
+---
+- true
+...
+test_run:cmd("start server test")
+---
+- true
+...
+test_run:cmd("switch test")
+---
+- true
+...
+_ = box.ctl.on_shutdown(function() print("on_shutdown 5") end)
+---
+...
+-- Check that we don't hang infinitely after os.exit()
+-- even if the following code doesn't yield.
+fiber = require('fiber')
+---
+...
+test_run:cmd("switch default")
+---
+- true
+...
+test_run:eval("test", "_ = fiber.new(function() os.exit() while true do end end)")
+---
+- []
+...
+fiber.sleep(0.1)
+---
+...
+-- The server should be already stopped by os.exit(),
+-- but start doesn't work without a prior call to stop.
+test_run:cmd("stop server test")
+---
+- true
+...
+test_run:cmd("start server test")
+---
+- true
+...
+test_run:wait_log('test', 'on_shutdown 5', nil, 5, {noreset=true})
+---
+- on_shutdown 5
+...
+-- make sure we exited because of os.exit(), not a signal.
+test_run:grep_log('test', 'signal', nil, {noreset=true})
+---
+- null
+...
+test_run:cmd("stop server test")
+---
+- true
+...
+test_run:cmd("cleanup server test")
+---
+- true
+...
+test_run:cmd("delete server test")
+---
+- true
+...
diff --git a/test/box/on_shutdown_triggers.test.lua b/test/box/on_shutdown_triggers.test.lua
new file mode 100644
index 000000000..9f8b0ae64
--- /dev/null
+++ b/test/box/on_shutdown_triggers.test.lua
@@ -0,0 +1,62 @@
+env = require('test_run')
+test_run = env.new()
+
+--
+-- gh-1607: on_shutdown triggers.
+--
+f = function() print('on_shutdown 1') end
+g = function() print('on_shutdown 2') end
+h = function() print('on_shutdown 3') end
+-- Check that on_shutdown triggers may yield
+-- and perform some complicated actions.
+fiber = require('fiber')
+test_run:cmd("setopt delimiter ';'")
+trig = function()
+    fiber.sleep(0.01)
+    fiber.yield()
+    box.schema.space.create("shutdown")
+    box.space.shutdown:create_index("pk")
+    box.space.shutdown:insert{1,2,3}
+    print('on_shutdown 4')
+end;
+test_run:cmd("setopt delimiter ''");
+_ = box.ctl.on_shutdown(f)
+_ = box.ctl.on_shutdown(g)
+-- Check that replacing triggers works
+_ = box.ctl.on_shutdown(h, g)
+_ = box.ctl.on_shutdown(trig)
+test_run:cmd('restart server default')
+test_run:grep_log('default', 'on_shutdown 1', nil, {noreset=true})
+test_run:grep_log('default', 'on_shutdown 2', nil, {noreset=true})
+test_run:grep_log('default', 'on_shutdown 3', nil, {noreset=true})
+test_run:grep_log('default', 'on_shutdown 4', nil, {noreset=true})
+box.space.shutdown:select{}
+box.space.shutdown:drop()
+
+-- Check that os.exit invokes on_shutdown triggers
+fiber = require("fiber")
+test_run:cmd("create server test with script='box/proxy.lua'")
+test_run:cmd("start server test")
+logfile = test_run:eval("test", "box.cfg.log")[1]
+test_run:cmd("stop server test")
+-- clean up any leftover logs
+require("fio").unlink(logfile)
+test_run:cmd("start server test")
+test_run:cmd("switch test")
+_ = box.ctl.on_shutdown(function() print("on_shutdown 5") end)
+-- Check that we don't hang infinitely after os.exit()
+-- even if the following code doesn't yield.
+fiber = require('fiber')
+test_run:cmd("switch default")
+test_run:eval("test", "_ = fiber.new(function() os.exit() while true do end end)")
+fiber.sleep(0.1)
+-- The server should be already stopped by os.exit(),
+-- but start doesn't work without a prior call to stop.
+test_run:cmd("stop server test")
+test_run:cmd("start server test")
+test_run:wait_log('test', 'on_shutdown 5', nil, 5, {noreset=true})
+-- make sure we exited because of os.exit(), not a signal.
+test_run:grep_log('test', 'signal', nil, {noreset=true})
+test_run:cmd("stop server test")
+test_run:cmd("cleanup server test")
+test_run:cmd("delete server test")
-- 
2.20.1 (Apple Git-117)

Re: [PATCH] test: rework box/misc test part regarding on_shutdown triggers

[Vladimir Davydov]

On Thu, Apr 11, 2019 at 04:37:20PM +0300, Serge Petrenko wrote:
> Rewrite the test so that it doesn't depend on timeouts, which can be
> exceeded under high load, too much and simplify it a little.
> 
> Also extract the whole part regarding on_shutdown triggers into a
> separate test, so that its sporadic failures would be easier to
> investigate.
> 
> Part of #4134
> ---
> https://github.com/tarantool/tarantool/issues/4134
> https://github.com/tarantool/tarantool/tree/sp/gh-4134-fix-on-shutdown-tests
> 
>  test/box/misc.result                   | 161 -------------------------
>  test/box/misc.test.lua                 |  62 ----------
>  test/box/on_shutdown_triggers.result   | 160 ++++++++++++++++++++++++
>  test/box/on_shutdown_triggers.test.lua |  62 ++++++++++

Better call simply box/on_shutdown.test.lua.

Anyway, this patch is impossible to review. Please split it in two - one
moves the test case to a separate file, another - fixes it.

I managed to extract the diff. Pasting it here for review.

> @@ -33,7 +33,7 @@
>  box.space.shutdown:select{}
>  box.space.shutdown:drop()
>  
> --- Check that os.exit invokes triggers
> +-- Check that os.exit invokes on_shutdown triggers
>  fiber = require("fiber")
>  test_run:cmd("create server test with script='box/proxy.lua'")
>  test_run:cmd("start server test")
> @@ -46,19 +46,17 @@
>  _ = box.ctl.on_shutdown(function() print("on_shutdown 5") end)
>  -- Check that we don't hang infinitely after os.exit()
>  -- even if the following code doesn't yield.
> -fiber = require("fiber")
> -_ = fiber.create(function() fiber.sleep(0.05) os.exit() while true do end end)
> +fiber = require('fiber')

This 'require' isn't needed anymore, no?

>  test_run:cmd("switch default")
> +test_run:eval("test", "_ = fiber.new(function() os.exit() while true do end end)")

This is supposed to stop the default instance?!

>  fiber.sleep(0.1)
>  -- The server should be already stopped by os.exit(),
>  -- but start doesn't work without a prior call to stop.
>  test_run:cmd("stop server test")
>  test_run:cmd("start server test")
> -test_run:cmd("switch test")
> -test_run:grep_log('test', 'on_shutdown 5', nil, {noreset=true})
> +test_run:wait_log('test', 'on_shutdown 5', nil, 5, {noreset=true})
>  -- make sure we exited because of os.exit(), not a signal.
>  test_run:grep_log('test', 'signal', nil, {noreset=true})
> -test_run:cmd("switch default")
>  test_run:cmd("stop server test")
>  test_run:cmd("cleanup server test")
>  test_run:cmd("delete server test")

Re: [tarantool-patches] Re: [PATCH] test: rework box/misc test part regarding on_shutdown triggers

[Serge Petrenko]

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

Hi! Thank you for review.

> 12 апр. 2019 г., в 17:00, Vladimir Davydov <vdavydov.dev@gmail.com> написал(а):
> 
> On Thu, Apr 11, 2019 at 04:37:20PM +0300, Serge Petrenko wrote:
>> Rewrite the test so that it doesn't depend on timeouts, which can be
>> exceeded under high load, too much and simplify it a little.
>> 
>> Also extract the whole part regarding on_shutdown triggers into a
>> separate test, so that its sporadic failures would be easier to
>> investigate.
>> 
>> Part of #4134
>> ---
>> https://github.com/tarantool/tarantool/issues/4134
>> https://github.com/tarantool/tarantool/tree/sp/gh-4134-fix-on-shutdown-tests
>> 
>> test/box/misc.result                   | 161 -------------------------
>> test/box/misc.test.lua                 |  62 ----------
>> test/box/on_shutdown_triggers.result   | 160 ++++++++++++++++++++++++
>> test/box/on_shutdown_triggers.test.lua |  62 ++++++++++
> 
> Better call simply box/on_shutdown.test.lua.
> 
> Anyway, this patch is impossible to review. Please split it in two - one
> moves the test case to a separate file, another - fixes it.

Will do.
> 
> I managed to extract the diff. Pasting it here for review.
> 
>> @@ -33,7 +33,7 @@
>> box.space.shutdown:select{}
>> box.space.shutdown:drop()
>> 
>> --- Check that os.exit invokes triggers
>> +-- Check that os.exit invokes on_shutdown triggers
>> fiber = require("fiber")
>> test_run:cmd("create server test with script='box/proxy.lua'")
>> test_run:cmd("start server test")
>> @@ -46,19 +46,17 @@
>> _ = box.ctl.on_shutdown(function() print("on_shutdown 5") end)
>> -- Check that we don't hang infinitely after os.exit()
>> -- even if the following code doesn't yield.
>> -fiber = require("fiber")
>> -_ = fiber.create(function() fiber.sleep(0.05) os.exit() while true do end end)
>> +fiber = require('fiber')
> 
> This 'require' isn't needed anymore, no?
> 
>> test_run:cmd("switch default")
>> +test_run:eval("test", "_ = fiber.new(function() os.exit() while true do end end)")
> 
> This is supposed to stop the default instance?!

No, it’s evaled on ’test’, which is the other instance. And that’s why we need the require(‘fiber’) above.
Anyways, will split the patch in two and resend.
> 
>> fiber.sleep(0.1)
>> -- The server should be already stopped by os.exit(),
>> -- but start doesn't work without a prior call to stop.
>> test_run:cmd("stop server test")
>> test_run:cmd("start server test")
>> -test_run:cmd("switch test")
>> -test_run:grep_log('test', 'on_shutdown 5', nil, {noreset=true})
>> +test_run:wait_log('test', 'on_shutdown 5', nil, 5, {noreset=true})
>> -- make sure we exited because of os.exit(), not a signal.
>> test_run:grep_log('test', 'signal', nil, {noreset=true})
>> -test_run:cmd("switch default")
>> test_run:cmd("stop server test")
>> test_run:cmd("cleanup server test")
>> test_run:cmd("delete server test")


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

Re: [tarantool-patches] Re: [PATCH] test: rework box/misc test part regarding on_shutdown triggers

[Vladimir Davydov]

On Fri, Apr 12, 2019 at 05:12:11PM +0300, Serge Petrenko wrote:
> >> test_run:cmd("switch default")
> >> +test_run:eval("test", "_ = fiber.new(function() os.exit() while true do end end)")
> > 
> > This is supposed to stop the default instance?!
> 
> No, it’s evaled on ’test’, which is the other instance. And that’s why we need the require(‘fiber’) above.
> Anyways, will split the patch in two and resend.

Missed the eval, sorry.

Re: [tarantool-patches] Re: [PATCH] test: rework box/misc test part regarding on_shutdown triggers

[Serge Petrenko]

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



> 12 апр. 2019 г., в 17:20, Vladimir Davydov <vdavydov.dev@gmail.com> написал(а):
> 
> On Fri, Apr 12, 2019 at 05:12:11PM +0300, Serge Petrenko wrote:
>>>> test_run:cmd("switch default")
>>>> +test_run:eval("test", "_ = fiber.new(function() os.exit() while true do end end)")
>>> 
>>> This is supposed to stop the default instance?!
>> 
>> No, it’s evaled on ’test’, which is the other instance. And that’s why we need the require(‘fiber’) above.
>> Anyways, will split the patch in two and resend.
> 
> Missed the eval, sorry.
It’s ok.
I’ve resent the patch. Please see '[PATCH v2 0/2] Rework on_shutdown test for better stability.'

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