[PATCH v2] box: add on_schema_init trigger

[PATCH v2] box: add on_schema_init trigger

[Serge Petrenko]

This patch introduces an on_schema_init trigger. The trigger may be set
before box.cfg() is called and is called during box.cfg() right after
prototypes of system spaces, such as _space, are created. This allows to
set triggers on system spaces before any other non-system data is
recovered. For example, it is possible to set an on_replace trigger on
_space, which will work even during recovery.

Part of #3159

@TarantoolBot document
Title: document box.ctl.on_schema_init triggers
on_schema_init triggers are set before the first call to box.cfg() and
are fired during box.cfg() before user data recovery start.

To set the trigger, say
```
box.ctl.on_schema_init(new_trig, old_trig)
```
where `old_trig` may be omitted. This will replace `old_trig` with `new_trig`.
Such triggers let you, for example, set triggers on system spaces before
recovery of any data, so that the triggers are fired even during
recovery.
For example, such triggers make it possible to change a specific space's
storage engine or make a replicated space replica-local on a freshly
bootstrapped replica.
If you want to change space's `space_name` storage engine to `vinyl`
, you may say:
```
function trig(old, new)
    if new[3] == 'space_name' and new[4] ~= 'vinyl' then
        return box.tuple.new{new[1], new[2], new[3], 'vinyl',
                             new[5], new[6], new[7]}
    end
end
```
Such a trigger may be set on `_space` as a `before_replace` trigger.
And thanks to `on_schema_init` triggers, it will happen before any
non-system spaces are recovered, so the trigger will work for all
user-created spaces:
```
box.ctl.on_schema_init(function()
    box.space._space:before_replace(trig)
end)
```
Note, that the above steps are done before initial `box.cfg{}` call.
Othervise the spaces will be already recovered by the time you set any
triggers.

Now you can say
`box.cfg{replication='master_uri', ...}`
And replica will have the space `space_name` with same contents, as on
master, but on `vinyl` storage engine.
---
https://github.com/tarantool/tarantool/tree/sp/gh-3159-on-schema-init
https://github.com/tarantool/tarantool/issues/3159

Changes in v2:
  - move trigger declaration to box/box.h
    remove box/schema.h dependency from
    box/lua/cfg.c
  - rename box-tap/ctl_on_schema_init.test.lua ->
    box-tap/on_schema_init.test.lua
  - add a test for changing storage engine and
    making a space local on replica.
  - extend the documentation request with a full
    usage example.

 src/box/box.h                               |   2 +
 src/box/lua/ctl.c                           |   7 ++
 src/box/schema.cc                           |   8 ++
 test/box-tap/on_schema_init.test.lua        |  31 +++++
 test/replication/on_schema_init.result      | 119 ++++++++++++++++++++
 test/replication/on_schema_init.test.lua    |  49 ++++++++
 test/replication/replica_on_schema_init.lua |  28 +++++
 test/replication/suite.cfg                  |   1 +
 8 files changed, 245 insertions(+)
 create mode 100755 test/box-tap/on_schema_init.test.lua
 create mode 100644 test/replication/on_schema_init.result
 create mode 100644 test/replication/on_schema_init.test.lua
 create mode 100644 test/replication/replica_on_schema_init.lua

diff --git a/src/box/box.h b/src/box/box.h
index 53d88ab71..ea6a11aee 100644
--- a/src/box/box.h
+++ b/src/box/box.h
@@ -67,6 +67,8 @@ extern const struct vclock *box_vclock;
 /** Invoked on box shutdown. */
 extern struct rlist box_on_shutdown;
 
+/** Triggers invoked after schema initialization. */
+extern struct rlist on_schema_init;
 /*
  * Initialize box library
  * @throws C++ exception
diff --git a/src/box/lua/ctl.c b/src/box/lua/ctl.c
index 7010be138..f90a9a973 100644
--- a/src/box/lua/ctl.c
+++ b/src/box/lua/ctl.c
@@ -71,10 +71,17 @@ lbox_ctl_on_shutdown(struct lua_State *L)
 	return lbox_trigger_reset(L, 2, &box_on_shutdown, NULL, NULL);
 }
 
+static int
+lbox_ctl_on_schema_init(struct lua_State *L)
+{
+	return lbox_trigger_reset(L, 2, &on_schema_init, NULL, 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},
+	{"on_schema_init", lbox_ctl_on_schema_init},
 	{NULL, NULL}
 };
 
diff --git a/src/box/schema.cc b/src/box/schema.cc
index 8625d92ea..f0fccf7f6 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -69,6 +69,8 @@ uint32_t schema_version = 0;
  */
 uint32_t space_cache_version = 0;
 
+struct rlist on_schema_init = RLIST_HEAD_INITIALIZER(on_schema_init);
+
 struct rlist on_alter_space = RLIST_HEAD_INITIALIZER(on_alter_space);
 struct rlist on_alter_sequence = RLIST_HEAD_INITIALIZER(on_alter_sequence);
 
@@ -500,6 +502,12 @@ schema_init()
 		init_system_space(space);
 		trigger_run_xc(&on_alter_space, space);
 	}
+
+	/*
+	 * Run the triggers right after creating all the system
+	 * space stubs.
+	 */
+	trigger_run(&on_schema_init, NULL);
 }
 
 void
diff --git a/test/box-tap/on_schema_init.test.lua b/test/box-tap/on_schema_init.test.lua
new file mode 100755
index 000000000..51b28ea08
--- /dev/null
+++ b/test/box-tap/on_schema_init.test.lua
@@ -0,0 +1,31 @@
+#!/usr/bin/env tarantool
+
+--
+-- gh-3159: test on_schema_init trigger
+--
+local tap = require('tap')
+local test = tap.test('on_schema_init')
+local str = ''
+test:plan(7)
+
+function testing_trig()
+    test:istable(box.space._space, 'system spaces are accessible')
+    test:is(type(box.space._space.before_replace), 'function', 'before_replace triggers')
+    test:is(type(box.space._space.on_replace), 'function', 'on_replace triggers')
+    test:is(type(box.space._space:on_replace(function() str = str.."_space:on_replace" end)),
+            'function', 'set on_replace trigger')
+    str = str..'on_schema_init'
+end
+
+trig = box.ctl.on_schema_init(testing_trig)
+test:is(type(trig), 'function', 'on_schema_init trigger set')
+
+box.cfg{log = 'tarantool.log'}
+test:like(str, 'on_schema_init', 'on_schema_init trigger works')
+str = ''
+box.schema.space.create("test")
+-- test that _space.on_replace trigger may be set in on_schema_init
+test:like(str, '_space:on_replace', 'can set on_replace')
+test:check()
+box.space.test:drop()
+os.exit(0)
diff --git a/test/replication/on_schema_init.result b/test/replication/on_schema_init.result
new file mode 100644
index 000000000..3f7ee0bd0
--- /dev/null
+++ b/test/replication/on_schema_init.result
@@ -0,0 +1,119 @@
+env = require('test_run')
+---
+...
+test_run = env.new()
+---
+...
+-- gh-3159: on_schema_init triggers
+-- the replica has set an on_schema_init trigger, which will set
+-- _space:before_replace triggers to change 'test_engine' space engine
+-- and 'test_local' space is_local flag when replication starts.
+test_run:cmd('create server replica with rpl_master=default, script="replication/replica_on_schema_init.lua"')
+---
+- true
+...
+test_engine = box.schema.space.create('test_engine', {engine='memtx'})
+---
+...
+test_local =  box.schema.space.create('test_local', {is_local=false})
+---
+...
+test_engine.engine
+---
+- memtx
+...
+test_local.is_local
+---
+- false
+...
+_ = test_engine:create_index("pk")
+---
+...
+_ = test_local:create_index("pk")
+---
+...
+test_engine:insert{1}
+---
+- [1]
+...
+test_local:insert{2}
+---
+- [2]
+...
+box.schema.user.grant('guest', 'replication')
+---
+...
+test_run:cmd('start server replica')
+---
+- true
+...
+test_run:cmd('switch replica')
+---
+- true
+...
+box.space.test_engine.engine
+---
+- vinyl
+...
+box.space.test_local.is_local
+---
+- true
+...
+box.space.test_engine:insert{3}
+---
+- [3]
+...
+box.space.test_local:insert{4}
+---
+- [4]
+...
+box.space.test_engine:select{}
+---
+- - [1]
+  - [3]
+...
+box.space.test_local:select{}
+---
+- - [2]
+  - [4]
+...
+test_run:cmd('switch default')
+---
+- true
+...
+test_run:cmd('set variable replica_port to "replica.listen"')
+---
+- true
+...
+box.cfg{replication=replica_port}
+---
+...
+test_engine:select{}
+---
+- - [1]
+  - [3]
+...
+-- the space truly became local on replica
+test_local:select{}
+---
+- - [2]
+...
+box.cfg{replication=nil}
+---
+...
+test_run:cmd('stop server replica with cleanup=1')
+---
+- true
+...
+test_run:cleanup_cluster()
+---
+...
+box.schema.user.revoke('guest', 'replication')
+---
+...
+test_engine:drop()
+---
+...
+test_local:drop()
+---
+...
diff --git a/test/replication/on_schema_init.test.lua b/test/replication/on_schema_init.test.lua
new file mode 100644
index 000000000..9bb9e4775
--- /dev/null
+++ b/test/replication/on_schema_init.test.lua
@@ -0,0 +1,49 @@
+env = require('test_run')
+test_run = env.new()
+
+-- gh-3159: on_schema_init triggers
+
+-- the replica has set an on_schema_init trigger, which will set
+-- _space:before_replace triggers to change 'test_engine' space engine
+-- and 'test_local' space is_local flag when replication starts.
+test_run:cmd('create server replica with rpl_master=default, script="replication/replica_on_schema_init.lua"')
+
+test_engine = box.schema.space.create('test_engine', {engine='memtx'})
+test_local =  box.schema.space.create('test_local', {is_local=false})
+test_engine.engine
+test_local.is_local
+
+_ = test_engine:create_index("pk")
+_ = test_local:create_index("pk")
+
+test_engine:insert{1}
+test_local:insert{2}
+
+box.schema.user.grant('guest', 'replication')
+
+test_run:cmd('start server replica')
+test_run:cmd('switch replica')
+
+box.space.test_engine.engine
+box.space.test_local.is_local
+
+box.space.test_engine:insert{3}
+box.space.test_local:insert{4}
+
+box.space.test_engine:select{}
+box.space.test_local:select{}
+
+test_run:cmd('switch default')
+
+test_run:cmd('set variable replica_port to "replica.listen"')
+box.cfg{replication=replica_port}
+test_engine:select{}
+-- the space truly became local on replica
+test_local:select{}
+
+box.cfg{replication=nil}
+test_run:cmd('stop server replica with cleanup=1')
+test_run:cleanup_cluster()
+box.schema.user.revoke('guest', 'replication')
+test_engine:drop()
+test_local:drop()
diff --git a/test/replication/replica_on_schema_init.lua b/test/replication/replica_on_schema_init.lua
new file mode 100644
index 000000000..663237757
--- /dev/null
+++ b/test/replication/replica_on_schema_init.lua
@@ -0,0 +1,28 @@
+#!/usr/bin/env tarantool
+
+function trig_local(old, new)
+    if new and new[3] == 'test_local' and new[6]['group_id'] ~= 1 then
+        return box.tuple.new({new[1],new[2],new[3],new[4],new[5],{group_id=1},new[7]})
+    end
+end
+
+function trig_engine(old, new)
+    if new and new[3] == 'test_engine' and new[4] ~= 'vinyl' then
+        return box.tuple.new({new[1],new[2],new[3],'vinyl',new[5],new[6],new[7]})
+    end
+end
+
+box.ctl.on_schema_init(function()
+    box.space._space:before_replace(trig_local)
+    box.space._space:before_replace(trig_engine)
+end)
+
+box.cfg({
+    listen              = os.getenv("LISTEN"),
+    replication         = os.getenv("MASTER"),
+    memtx_memory        = 107374182,
+    replication_timeout = 0.1,
+    replication_connect_timeout = 0.5,
+})
+
+require('console').listen(os.getenv('ADMIN'))
diff --git a/test/replication/suite.cfg b/test/replication/suite.cfg
index fc7c0c464..5e8809731 100644
--- a/test/replication/suite.cfg
+++ b/test/replication/suite.cfg
@@ -8,6 +8,7 @@
     "rebootstrap.test.lua": {},
     "wal_rw_stress.test.lua": {},
     "force_recovery.test.lua": {},
+    "on_schema_init.test.lua": {},
     "*": {
         "memtx": {"engine": "memtx"},
         "vinyl": {"engine": "vinyl"}
-- 
2.17.2 (Apple Git-113)

Re: [PATCH v2] box: add on_schema_init trigger

[Konstantin Osipov]

* Serge Petrenko <sergepetrenko@tarantool.org> [19/03/05 18:57]:
> If you want to change space's `space_name` storage engine to `vinyl`
> , you may say:
> ```
> function trig(old, new)
>     if new[3] == 'space_name' and new[4] ~= 'vinyl' then
>         return box.tuple.new{new[1], new[2], new[3], 'vinyl',
>                              new[5], new[6], new[7]}
>     end
> end
> ```

Please access tuple fields by their names ;)


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

Re: [PATCH v2] box: add on_schema_init trigger

[Konstantin Osipov]

* Serge Petrenko <sergepetrenko@tarantool.org> [19/03/05 18:57]:
> This patch introduces an on_schema_init trigger. The trigger may be set
> before box.cfg() is called and is called during box.cfg() right after
> prototypes of system spaces, such as _space, are created. This allows to
> set triggers on system spaces before any other non-system data is
> recovered. For example, it is possible to set an on_replace trigger on
> _space, which will work even during recovery.

Now lgtm, Vova, please do your own review and push.


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

Re: [PATCH v2] box: add on_schema_init trigger

[Vladimir Davydov]

On Tue, Mar 05, 2019 at 06:53:09PM +0300, Serge Petrenko wrote:
> If you want to change space's `space_name` storage engine to `vinyl`
> , you may say:
> ```
> function trig(old, new)
>     if new[3] == 'space_name' and new[4] ~= 'vinyl' then
>         return box.tuple.new{new[1], new[2], new[3], 'vinyl',
>                              new[5], new[6], new[7]}
>     end
> end
> ```

You could use new:update{{'=', 4, 'vinyl'}} instead.

> diff --git a/src/box/box.h b/src/box/box.h
> index 53d88ab71..ea6a11aee 100644
> --- a/src/box/box.h
> +++ b/src/box/box.h
> @@ -67,6 +67,8 @@ extern const struct vclock *box_vclock;
>  /** Invoked on box shutdown. */
>  extern struct rlist box_on_shutdown;
>  
> +/** Triggers invoked after schema initialization. */
> +extern struct rlist on_schema_init;

I moved it back to schema.h (Kostja agreed that it'd be better).

Pushed to 2.1 with the following changes:

diff --git a/src/box/box.h b/src/box/box.h
index ea6a11ae..53d88ab7 100644
--- a/src/box/box.h
+++ b/src/box/box.h
@@ -67,8 +67,6 @@ extern const struct vclock *box_vclock;
 /** Invoked on box shutdown. */
 extern struct rlist box_on_shutdown;
 
-/** Triggers invoked after schema initialization. */
-extern struct rlist on_schema_init;
 /*
  * Initialize box library
  * @throws C++ exception
diff --git a/src/box/lua/ctl.c b/src/box/lua/ctl.c
index f90a9a97..85ed30c5 100644
--- a/src/box/lua/ctl.c
+++ b/src/box/lua/ctl.c
@@ -40,6 +40,7 @@
 #include "lua/trigger.h"
 
 #include "box/box.h"
+#include "box/schema.h"
 
 static int
 lbox_ctl_wait_ro(struct lua_State *L)
diff --git a/src/box/schema.cc b/src/box/schema.cc
index f0fccf7f..74d70d8d 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -70,7 +70,6 @@ uint32_t schema_version = 0;
 uint32_t space_cache_version = 0;
 
 struct rlist on_schema_init = RLIST_HEAD_INITIALIZER(on_schema_init);
-
 struct rlist on_alter_space = RLIST_HEAD_INITIALIZER(on_alter_space);
 struct rlist on_alter_sequence = RLIST_HEAD_INITIALIZER(on_alter_sequence);
 
diff --git a/src/box/schema.h b/src/box/schema.h
index f3df08b4..6f9a9611 100644
--- a/src/box/schema.h
+++ b/src/box/schema.h
@@ -44,6 +44,9 @@ extern "C" {
 extern uint32_t schema_version;
 extern uint32_t space_cache_version;
 
+/** Triggers invoked after schema initialization. */
+extern struct rlist on_schema_init;
+
 /**
  * Lock of schema modification
  */
diff --git a/test/replication/replica_on_schema_init.lua b/test/replication/replica_on_schema_init.lua
index 66323775..8a221681 100644
--- a/test/replication/replica_on_schema_init.lua
+++ b/test/replication/replica_on_schema_init.lua
@@ -2,13 +2,13 @@
 
 function trig_local(old, new)
     if new and new[3] == 'test_local' and new[6]['group_id'] ~= 1 then
-        return box.tuple.new({new[1],new[2],new[3],new[4],new[5],{group_id=1},new[7]})
+        return new:update{{'=', 6, {group_id = 1}}}
     end
 end
 
 function trig_engine(old, new)
     if new and new[3] == 'test_engine' and new[4] ~= 'vinyl' then
-        return box.tuple.new({new[1],new[2],new[3],'vinyl',new[5],new[6],new[7]})
+        return new:update{{'=', 4, 'vinyl'}}
     end
 end
 
@@ -20,9 +20,6 @@ end)
 box.cfg({
     listen              = os.getenv("LISTEN"),
     replication         = os.getenv("MASTER"),
-    memtx_memory        = 107374182,
-    replication_timeout = 0.1,
-    replication_connect_timeout = 0.5,
 })
 
 require('console').listen(os.getenv('ADMIN'))

Re: [PATCH v2] box: add on_schema_init trigger

[Vladimir Davydov]

On Tue, Mar 05, 2019 at 09:59:06PM +0300, Konstantin Osipov wrote:
> * Serge Petrenko <sergepetrenko@tarantool.org> [19/03/05 18:57]:
> > If you want to change space's `space_name` storage engine to `vinyl`
> > , you may say:
> > ```
> > function trig(old, new)
> >     if new[3] == 'space_name' and new[4] ~= 'vinyl' then
> >         return box.tuple.new{new[1], new[2], new[3], 'vinyl',
> >                              new[5], new[6], new[7]}
> >     end
> > end
> > ```
> 
> Please access tuple fields by their names ;)

Unfortunately, we can't - formats are not set in schema_init(), they are
recovered from the snapshot. Probably, we can fix that.