Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v1 0/2] tarantoolctl: add signal in stop/restart options
@ 2020-11-11  5:21 Alexander V. Tikhonov
  2020-11-11  5:21 ` [Tarantool-patches] [PATCH v1 1/2] " Alexander V. Tikhonov
  2020-11-11  5:21 ` [Tarantool-patches] [PATCH v1 2/2] Update tests for the new tarantoolctl format Alexander V. Tikhonov
  0 siblings, 2 replies; 3+ messages in thread
From: Alexander V. Tikhonov @ 2020-11-11  5:21 UTC (permalink / raw)
  To: Alexander Turenko, Kirill Yukhin; +Cc: tarantool-patches

Found that some of the instances could not be killed with SIGTERM signal
and it really needed SIGKILL signal to be sure that the instance would
be terminated. Otherwise some tests may fail, like:

  No output during 120 seconds. Will abort after 120 seconds without output. List of workers not reporting the status:
  - 001_engine [engine/json.test.lua, memtx] at var/001_engine/json.result:598
  Test hung! Result content mismatch:
  --- engine/json.result        Fri Nov  6 08:35:45 2020
  +++ var/001_engine/json.result        Mon Nov  9 03:48:40 2020
  @@ -596,248 +596,3 @@
   - ok
   ...
   test_run:cmd("restart server default")
  -engine = test_run:get_cfg('engine')
  ----
  -…

Test run uses 'tarantoolctl' tool for commands in format "restart server
default", and 'tarantoolctl' had only single way to kill it using
SIGTERM signal. To implement ability to use it with SIGKILL was added
additional non-mandatory option to stop and restart 'tarantoolctl'
commands, like:

  tarantoolctl stop <instance> [--signal=SIGKILL]
  tarantoolctl restart <instance> [--signal=SIGKILL]

This options didn't change the backward compatibility of 'tarantoolctl'
tool, but gave the tests ability to stop and restart instances in the
following way:

  test_run:cmd("stop server default with signal=KILL")
  test_run:cmd("restart server default with signal=KILL")

It helped to fix the issue with hanging instances in the needed tests.

After test-run changed for the new 'tarantoolctl' format for 'restart'
and 'stop' commands some tests had to be updated due to checked it.

Closes #5510

Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-5510-tarantoolctl-signals
Issue: https://github.com/tarantool/tarantool/issues/5510

Alexander V. Tikhonov (2):
  tarantoolctl: add signal in stop/restart options
  Update tests for the new tarantoolctl format

 extra/dist/tarantoolctl.in                 | 22 ++++++++++++++++++----
 test/app/cmdline.result                    |  6 +++++-
 test/app/cmdline.test.lua                  |  3 ++-
 test/replication/bootstrap_leader.result   |  2 +-
 test/replication/bootstrap_leader.test.lua |  2 +-
 5 files changed, 27 insertions(+), 8 deletions(-)

-- 
2.25.1

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

* [Tarantool-patches] [PATCH v1 1/2] tarantoolctl: add signal in stop/restart options
  2020-11-11  5:21 [Tarantool-patches] [PATCH v1 0/2] tarantoolctl: add signal in stop/restart options Alexander V. Tikhonov
@ 2020-11-11  5:21 ` Alexander V. Tikhonov
  2020-11-11  5:21 ` [Tarantool-patches] [PATCH v1 2/2] Update tests for the new tarantoolctl format Alexander V. Tikhonov
  1 sibling, 0 replies; 3+ messages in thread
From: Alexander V. Tikhonov @ 2020-11-11  5:21 UTC (permalink / raw)
  To: Alexander Turenko, Kirill Yukhin; +Cc: tarantool-patches

Found that some of the instances could not be killed with SIGTERM signal
and it really needed SIGKILL signal to be sure that the instance would
be terminated. Otherwise some tests may fail, like:

  No output during 120 seconds. Will abort after 120 seconds without
output. List of workers not reporting the status:
  - 001_engine [engine/json.test.lua, memtx] at
var/001_engine/json.result:598
  Test hung! Result content mismatch:
  --- engine/json.result	Fri Nov  6 08:35:45 2020
  +++ var/001_engine/json.result	Mon Nov  9 03:48:40 2020
  @@ -596,248 +596,3 @@
   - ok
   ...
   test_run:cmd("restart server default")
  -engine = test_run:get_cfg('engine')
  ----
  -…

Test run uses 'tarantoolctl' tool for commands in format "restart server
default", and 'tarantoolctl' had only single way to kill it using
SIGTERM signal. To implement ability to use it with SIGKILL was added
additional non-mandatory option to stop and restart 'tarantoolctl'
commands, like:

  tarantoolctl stop <instance> [--signal=SIGKILL]
  tarantoolctl restart <instance> [--signal=SIGKILL]

This options didn't change the backward compatibility of 'tarantoolctl'
tool, but gave the tests ability to stop and restart instances in the
following way:

  test_run:cmd("stop server default with signal=KILL")
  test_run:cmd("restart server default with signal=KILL")

It helped to fix the issue with hanging instances in the needed tests.

Closes #5510
---
 extra/dist/tarantoolctl.in | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/extra/dist/tarantoolctl.in b/extra/dist/tarantoolctl.in
index 0726e7f46..1666b1005 100755
--- a/extra/dist/tarantoolctl.in
+++ b/extra/dist/tarantoolctl.in
@@ -47,6 +47,7 @@ local positional_arguments
 local keyword_arguments
 local lua_arguments = arg
 local language
+local signal
 
 -- function for printing usage reference
 local usage
@@ -162,6 +163,10 @@ local function load_default_file(default_file)
         os.exit(1)
     end
 
+    -- SIGTERM by default
+    signal = d.signal or 15
+    d.signal = nil
+
     default_cfg = d
 
     if not usermode then
@@ -595,8 +600,10 @@ local function stop()
         return 1
     end
 
-    if ffi.C.kill(pid, 15) < 0 then
-        log.error("Can't kill process %d: %s", pid, errno.strerror())
+    signal = keyword_arguments.signal or signal
+    if ffi.C.kill(pid, 0) and ffi.C.kill(pid, signal) < 0 then
+        log.error("Can't kill process %d with signal %d: %s",
+                  pid, signal, errno.strerror())
         fio.unlink(pid_file)
         return 1
     end
@@ -1010,11 +1017,14 @@ local commands = setmetatable({
         }
     }, stop = {
         func = exit_wrapper(stop), process = process_local, help = {
-            header = "%s stop INSTANCE",
+            header = "%s stop INSTANCE [--signal=signal]",
             linkmode = "%s stop",
             description =
 [=[
         Stop a Tarantool instance.
+
+        Supported options:
+        * --signal=signal use to the stop the instance.
 ]=],
             weight = 20,
             deprecated = false,
@@ -1065,11 +1075,14 @@ local commands = setmetatable({
         }
     }, restart = {
         func = restart, process = process_local, help = {
-            header = "%s restart INSTANCE",
+            header = "%s restart INSTANCE [--signal=signal]",
             linkmode = "%s restart",
             description =
 [=[
         Stop and start a Tarantool instance.
+
+        Supported options:
+        * --signal=signal use to the stop the instance.
 ]=],
             weight = 40,
             deprecated = false,
@@ -1288,6 +1301,7 @@ local function populate_arguments()
         { 'format',      'string'  },
         { 'replica',     'number+' },
         { 'language',    'string'  },
+        { 'signal',      'number'  },
     })
 
     local cmd_name
-- 
2.25.1

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

* [Tarantool-patches] [PATCH v1 2/2] Update tests for the new tarantoolctl format
  2020-11-11  5:21 [Tarantool-patches] [PATCH v1 0/2] tarantoolctl: add signal in stop/restart options Alexander V. Tikhonov
  2020-11-11  5:21 ` [Tarantool-patches] [PATCH v1 1/2] " Alexander V. Tikhonov
@ 2020-11-11  5:21 ` Alexander V. Tikhonov
  1 sibling, 0 replies; 3+ messages in thread
From: Alexander V. Tikhonov @ 2020-11-11  5:21 UTC (permalink / raw)
  To: Alexander Turenko, Kirill Yukhin; +Cc: tarantool-patches

After test-run changed for the new 'tarantoolctl' format for 'restart'
and 'stop' commands some tests had to be updated due to checked it.
---
 test/app/cmdline.result                    | 6 +++++-
 test/app/cmdline.test.lua                  | 3 ++-
 test/replication/bootstrap_leader.result   | 2 +-
 test/replication/bootstrap_leader.test.lua | 2 +-
 4 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/test/app/cmdline.result b/test/app/cmdline.result
index facb915e2..291d23844 100644
--- a/test/app/cmdline.result
+++ b/test/app/cmdline.result
@@ -18,7 +18,11 @@ string.match(arg[-1], '/tarantool$') ~= nil
 ---
 - true
 ...
-string.match(arg[2], 'app%.lua$') ~= nil
+string.match(arg[0], 'app%.lua$') ~= nil
+---
+- true
+...
+string.match(arg[3], '--signal=9$') ~= nil
 ---
 - true
 ...
diff --git a/test/app/cmdline.test.lua b/test/app/cmdline.test.lua
index 316f761fc..44a3dd7bb 100644
--- a/test/app/cmdline.test.lua
+++ b/test/app/cmdline.test.lua
@@ -4,7 +4,8 @@ string.match(arg[-1], '^/') ~= nil
 string.match(arg[0], '^/') == nil
 
 string.match(arg[-1], '/tarantool$') ~= nil
-string.match(arg[2], 'app%.lua$') ~= nil
+string.match(arg[0], 'app%.lua$') ~= nil
+string.match(arg[3], '--signal=9$') ~= nil
 
 io.type( io.open(arg[-1]) )
 io.type( io.open(arg[0]) )
diff --git a/test/replication/bootstrap_leader.result b/test/replication/bootstrap_leader.result
index 7d1a33d8e..a14c863ab 100644
--- a/test/replication/bootstrap_leader.result
+++ b/test/replication/bootstrap_leader.result
@@ -18,7 +18,7 @@ end;
  | ---
  | ...
 
-test_run:cmd("start server replica1 with wait_load=True, wait=True");
+test_run:cmd("start server replica1 with args='', wait_load=True, wait=True");
  | ---
  | - true
  | ...
diff --git a/test/replication/bootstrap_leader.test.lua b/test/replication/bootstrap_leader.test.lua
index 984a82b8e..48260e8bd 100644
--- a/test/replication/bootstrap_leader.test.lua
+++ b/test/replication/bootstrap_leader.test.lua
@@ -8,7 +8,7 @@ for i = 1,3 do
     test_run:cmd(string.format(create_server_cmd, i, i))
 end;
 
-test_run:cmd("start server replica1 with wait_load=True, wait=True");
+test_run:cmd("start server replica1 with args='', wait_load=True, wait=True");
 test_run:cmd("start server replica2 with args='1,2,3 1.0 100500 0.1', wait_load=False, wait=False");
 test_run:cmd("start server replica3 with args='1,2,3 0.1 0.5 100500', wait_load=True, wait=True");
 
-- 
2.25.1

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

end of thread, other threads:[~2020-11-11  5:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-11  5:21 [Tarantool-patches] [PATCH v1 0/2] tarantoolctl: add signal in stop/restart options Alexander V. Tikhonov
2020-11-11  5:21 ` [Tarantool-patches] [PATCH v1 1/2] " Alexander V. Tikhonov
2020-11-11  5:21 ` [Tarantool-patches] [PATCH v1 2/2] Update tests for the new tarantoolctl format Alexander V. Tikhonov

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