Tarantool development patches archive
 help / color / mirror / Atom feed
From: Konstantin Osipov <kostja@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Serge Petrenko <sergepetrenko@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH 2/2] box: introduce on_shutdown triggers to lua
Date: Fri, 23 Nov 2018 13:37:04 +0300	[thread overview]
Message-ID: <20181123103704.GB10757@chai> (raw)
In-Reply-To: <a6bd6ab0063d1808d001bb1e10f42043ef629fea.1542906167.git.sergepetrenko@tarantool.org>

* Serge Petrenko <sergepetrenko@tarantool.org> [18/11/22 20:21]:
> Make it possible to register on_shutdown triggers from lua via
> box.ctl.on_shutdown()

OK, this is a nice step forward, can we add shutdown reason to the
event and marshal it to Lua? 

Why are you calling the trigger only in case box is configured?
Why should it matter?

The ticket is missing a docbot plea.

What if on_shutdown trigger yields? We either should document that
it can't yield or test that it can yield. Besides, it can do some
very nasty things, like use 'box', or sockets, or seep. We should
(if possible) test and document what it can and cannot do.

> 
> Part of #1607
> ---
>  src/box/lua/ctl.c      | 21 +++++++++++++++++++++
>  test/box/misc.result   | 34 ++++++++++++++++++++++++++++++++++
>  test/box/misc.test.lua | 14 ++++++++++++++
>  3 files changed, 69 insertions(+)
> 
> diff --git a/src/box/lua/ctl.c b/src/box/lua/ctl.c
> index 9a105ed5c..dad39515c 100644
> --- a/src/box/lua/ctl.c
> +++ b/src/box/lua/ctl.c
> @@ -37,6 +37,7 @@
>  #include <lualib.h>
>  
>  #include "lua/utils.h"
> +#include "lua/trigger.h"
>  
>  #include "box/box.h"
>  
> @@ -64,9 +65,29 @@ lbox_ctl_wait_rw(struct lua_State *L)
>  	return 0;
>  }
>  
> +/*
> + * This is a placeholder. It will be used when we decide to pass
> + * a signal which caused termination to the trigger.
> + */
> +static int
> +lbox_push_on_shutdown(struct lua_State *L, void *event)
> +{
> +	(void)L;
> +	(void)event;
> +	return 0;
> +}
> +
> +static int
> +lbox_ctl_on_shutdown(struct lua_State *L)
> +{
> +	return lbox_trigger_reset(L, 2, &on_shutdown, lbox_push_on_shutdown,
> +				  NULL);
> +}
> +
>  static const struct luaL_Reg lbox_ctl_lib[] = {
>  	{"wait_ro", lbox_ctl_wait_ro},
>  	{"wait_rw", lbox_ctl_wait_rw},
> +	{"on_shutdown", lbox_ctl_on_shutdown},
>  	{NULL, NULL}
>  };
>  
> diff --git a/test/box/misc.result b/test/box/misc.result
> index 4ee4797d0..fd55b8cf1 100644
> --- a/test/box/misc.result
> +++ b/test/box/misc.result
> @@ -1205,3 +1205,37 @@ 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
> +---
> +...
> +_ = box.ctl.on_shutdown(f)
> +---
> +...
> +_ = box.ctl.on_shutdown(g)
> +---
> +...
> +_ = box.ctl.on_shutdown(h, g)
> +---
> +...
> +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
> +...
> diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
> index ee81c7be1..a4b38b403 100644
> --- a/test/box/misc.test.lua
> +++ b/test/box/misc.test.lua
> @@ -342,3 +342,17 @@ 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
> +_ = box.ctl.on_shutdown(f)
> +_ = box.ctl.on_shutdown(g)
> +_ = box.ctl.on_shutdown(h, g)
> +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})
> -- 
> 2.17.2 (Apple Git-113)
> 

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

      reply	other threads:[~2018-11-23 10:37 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-22 17:20 [tarantool-patches] [PATCH 0/2] box: implement on_shutdown triggers Serge Petrenko
2018-11-22 17:20 ` [tarantool-patches] [PATCH 1/2] " Serge Petrenko
2018-11-22 17:20 ` [tarantool-patches] [PATCH 2/2] box: introduce on_shutdown triggers to lua Serge Petrenko
2018-11-23 10:37   ` Konstantin Osipov [this message]

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=20181123103704.GB10757@chai \
    --to=kostja@tarantool.org \
    --cc=sergepetrenko@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH 2/2] box: introduce on_shutdown triggers to lua' \
    /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