[Tarantool-patches] [PATCH 0/3] Fix upgrade from 2.1 sequence

[Tarantool-patches] [PATCH 0/3] Fix upgrade from 2.1 sequence

[Vladislav Shpilevoy]

The patchset fixes inability to upgrade from 2.1 if there is an
automatically generated sequence. The problem was that upgrade
tried to update a tuple in _space_sequence in-place, calling
space:update() method. But it was not properly supported for
_space_sequence. The patchset turns it into delete + insert, and
bans updates for this space.

Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4771-2.2-upgrade
Issue: https://github.com/tarantool/tarantool/issues/4771

@ChangeLog
- Fixed inability to upgrade from 2.1, if there was an
  automatically generated sequence (gh-4771).

Vladislav Shpilevoy (3):
  upgrade: add missing sys triggers off and erasure
  box: forbid to update/replace _space_sequence
  upgrade: fix generated sequence upgrade from 2.1

 src/box/alter.cc                              |   8 +-
 src/box/lua/upgrade.lua                       |  47 ++--
 test/box/sequence.result                      |  19 ++
 test/box/sequence.test.lua                    |   9 +
 test/xlog/gh-4771-upgrade.result              |  78 ++++++
 test/xlog/gh-4771-upgrade.test.lua            |  24 ++
 test/xlog/suite.cfg                           |   5 -
 test/xlog/suite.ini                           |   3 +-
 test/xlog/upgrade.result                      | 265 ------------------
 test/xlog/upgrade.test.lua                    |  46 ---
 .../00000000000000000014.snap                 | Bin 0 -> 4622 bytes
 .../2.1.3/gh-4771-upgrade-sequence/fill.lua   |  14 +
 12 files changed, 172 insertions(+), 346 deletions(-)
 create mode 100644 test/xlog/gh-4771-upgrade.result
 create mode 100644 test/xlog/gh-4771-upgrade.test.lua
 delete mode 100644 test/xlog/suite.cfg
 delete mode 100644 test/xlog/upgrade.result
 delete mode 100644 test/xlog/upgrade.test.lua
 create mode 100644 test/xlog/upgrade/2.1.3/gh-4771-upgrade-sequence/00000000000000000014.snap
 create mode 100644 test/xlog/upgrade/2.1.3/gh-4771-upgrade-sequence/fill.lua

-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 1/3] upgrade: add missing sys triggers off and erasure

[Vladislav Shpilevoy]

box.internal.bootstrap() before doing anything turns off system
space triggers, because it is likely to do some hard changes
violating existing rules. And eliminates data from all system
spaces to fill it from the scratch.

Each time when a new space is added, its erasure and turning off
its triggers should have been called explicitly here. As a result
it was not done sometimes, by accident. For example, triggers
were not turned off for _sequence_data, _sequence,
_space_sequence.

Content removal wasn't done for _space_sequence.

The patch makes a generic solution which does not require manual
patching of trigger manipulation and truncation anymore.

The bug was discovered while working on #4771, although it is not
related.
---
 src/box/lua/upgrade.lua | 40 +++++++++++++++-------------------------
 1 file changed, 15 insertions(+), 25 deletions(-)

diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index c69b6b543..bd28c1001 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -63,18 +63,21 @@ local function truncate(space)
     end
 end
 
+local function foreach_system_space(cb)
+    local max = box.schema.SYSTEM_ID_MAX
+    for id, space in pairs(box.space) do
+        if type(id) == 'number' and
+           (space.engine == 'memtx' or space.engine == 'vinyl') then
+            if id > max then
+                break
+            end
+            cb(space)
+        end
+    end
+end
+
 local function set_system_triggers(val)
-    box.space._space:run_triggers(val)
-    box.space._index:run_triggers(val)
-    box.space._user:run_triggers(val)
-    box.space._func:run_triggers(val)
-    box.space._priv:run_triggers(val)
-    box.space._trigger:run_triggers(val)
-    box.space._collation:run_triggers(val)
-    box.space._schema:run_triggers(val)
-    box.space._cluster:run_triggers(val)
-    box.space._fk_constraint:run_triggers(val)
-    box.space._ck_constraint:run_triggers(val)
+    foreach_system_space(function(s) s:run_triggers(val) end)
 end
 
 --------------------------------------------------------------------------------
@@ -82,20 +85,7 @@ end
 --------------------------------------------------------------------------------
 
 local function erase()
-    truncate(box.space._space)
-    truncate(box.space._index)
-    truncate(box.space._user)
-    truncate(box.space._func)
-    truncate(box.space._priv)
-    truncate(box.space._sequence_data)
-    truncate(box.space._sequence)
-    truncate(box.space._truncate)
-    truncate(box.space._collation)
-    truncate(box.space._trigger)
-    truncate(box.space._schema)
-    truncate(box.space._cluster)
-    truncate(box.space._fk_constraint)
-    truncate(box.space._ck_constraint)
+    foreach_system_space(function(s) truncate(s) end)
 end
 
 local function create_sysview(source_id, target_id)
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 2/3] box: forbid to update/replace _space_sequence

[Vladislav Shpilevoy]

Anyway this does not work for generated sequences. A proper
support of update would complicate the code and won't give
anything useful.

Part of #4771
---
 src/box/alter.cc           |  8 ++++++--
 test/box/sequence.result   | 19 +++++++++++++++++++
 test/box/sequence.test.lua |  9 +++++++++
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 64af71c61..50121c79c 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -4713,9 +4713,13 @@ on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event)
 		return -1;
 	}
 
+	if ((stmt->new_tuple != NULL) == (stmt->old_tuple != NULL)) {
+		diag_set(ClientError, ER_UNSUPPORTED,
+			 "space \"_space_sequence\"", "update");
+		return -1;
+	}
+
 	enum priv_type priv_type = stmt->new_tuple ? PRIV_C : PRIV_D;
-	if (stmt->new_tuple && stmt->old_tuple)
-		priv_type = PRIV_A;
 
 	/* Check we have the correct access type on the sequence.  * */
 	if (is_generated || !stmt->new_tuple) {
diff --git a/test/box/sequence.result b/test/box/sequence.result
index 917952909..0a6cfee2c 100644
--- a/test/box/sequence.result
+++ b/test/box/sequence.result
@@ -2270,3 +2270,22 @@ sq:next() -- 2
 sq:drop()
 ---
 ...
+--
+-- Update on _space_sequence is forbidden.
+--
+s = box.schema.create_space('test')
+---
+...
+pk = s:create_index('pk', {sequence = true})
+---
+...
+t = box.space._space_sequence:get({s.id})
+---
+...
+box.space._space_sequence:update({s.id}, {{'=', 2, t[2]}})
+---
+- error: space "_space_sequence" does not support update
+...
+s:drop()
+---
+...
diff --git a/test/box/sequence.test.lua b/test/box/sequence.test.lua
index b0ec020ed..8e00571e5 100644
--- a/test/box/sequence.test.lua
+++ b/test/box/sequence.test.lua
@@ -771,3 +771,12 @@ sq:next() -- 1
 box.begin() box.space._sequence_data:delete{sq.id} box.rollback()
 sq:next() -- 2
 sq:drop()
+
+--
+-- Update on _space_sequence is forbidden.
+--
+s = box.schema.create_space('test')
+pk = s:create_index('pk', {sequence = true})
+t = box.space._space_sequence:get({s.id})
+box.space._space_sequence:update({s.id}, {{'=', 2, t[2]}})
+s:drop()
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 3/3] upgrade: fix generated sequence upgrade from 2.1

[Vladislav Shpilevoy]

The bug was in an attempt to update a record in _space_sequence
in-place, to add field path and number. This was not properly
supported by the system space's trigger, and was banned in the
previous patch of this series.

But delete + tuple update + insert work fine. The patch uses them.

To test it the old disabled and heavily outdated
xlog/upgrade.test.lua was replaced with a smaller analogue, which
is supposed to be created separately for each upgrade bug.
According to the new policy of creating test files.

The patch tries to make it easy to add new upgrade tests and
snapshots. A new test should consist of fill.lua script to
populate spaces, snapshot, needed xlogs, and a .test.lua file.
Fill script and binaries should be in the same folder as test file
name, which is located in version folder. Like this:

 xlog/
 |
 + <test_name>.test.lua
 |
 +- upgrade/
    |
    +- <version>/
    |   |
    |   +-<test_name>/
    |     |
    |     +- fill.lua
    |     +- *.snap
    |     +- *.xlog

Version is supposed to say explicitly what a version files in
there have.

Closes #4771
---
 src/box/lua/upgrade.lua                       |   7 +-
 test/xlog/gh-4771-upgrade.result              |  78 ++++++
 test/xlog/gh-4771-upgrade.test.lua            |  24 ++
 test/xlog/suite.cfg                           |   5 -
 test/xlog/suite.ini                           |   3 +-
 test/xlog/upgrade.result                      | 265 ------------------
 test/xlog/upgrade.test.lua                    |  46 ---
 .../00000000000000000014.snap                 | Bin 0 -> 4622 bytes
 .../2.1.3/gh-4771-upgrade-sequence/fill.lua   |  14 +
 9 files changed, 123 insertions(+), 319 deletions(-)
 create mode 100644 test/xlog/gh-4771-upgrade.result
 create mode 100644 test/xlog/gh-4771-upgrade.test.lua
 delete mode 100644 test/xlog/suite.cfg
 delete mode 100644 test/xlog/upgrade.result
 delete mode 100644 test/xlog/upgrade.test.lua
 create mode 100644 test/xlog/upgrade/2.1.3/gh-4771-upgrade-sequence/00000000000000000014.snap
 create mode 100644 test/xlog/upgrade/2.1.3/gh-4771-upgrade-sequence/fill.lua

diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index bd28c1001..4e23aa448 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -760,7 +760,12 @@ local function upgrade_sequence_to_2_2_1()
         local part = pk[6][1]
         local field = part.field or part[1]
         local path = part.path or ''
-        _space_sequence:update(v[1], {{'!', 4, field}, {'!', 5, path}})
+        local t = _space_sequence:get(v[1])
+        -- Update in-place is banned due to complexity of its
+        -- handling. Delete + insert still work.
+        t = t:update({{'!', 4, field}, {'!', 5, path}})
+        _space_sequence:delete({v[1]})
+        _space_sequence:insert(t)
         ::continue::
     end
     local format = _space_sequence:format()
diff --git a/test/xlog/gh-4771-upgrade.result b/test/xlog/gh-4771-upgrade.result
new file mode 100644
index 000000000..032ed993a
--- /dev/null
+++ b/test/xlog/gh-4771-upgrade.result
@@ -0,0 +1,78 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+
+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", '..		\
+             'workdir="xlog/upgrade/2.1.3/gh-4771-upgrade-sequence"')
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server upgrade')
+ | ---
+ | - true
+ | ...
+test_run:switch('upgrade')
+ | ---
+ | - true
+ | ...
+
+box.schema.upgrade()
+ | ---
+ | ...
+
+s = box.space.test1
+ | ---
+ | ...
+box.space._sequence:select{}
+ | ---
+ | - - [1, 1, 'test1_seq', 1, 1, 9223372036854775807, 1, 0, false]
+ |   - [2, 1, 'seq2', 1, 1, 9223372036854775807, 1, 0, false]
+ |   - [3, 1, 'seq3', 1, 1, 9223372036854775807, 1, 0, false]
+ | ...
+box.space._sequence_data:select{}
+ | ---
+ | - - [1, 1]
+ |   - [2, 1]
+ |   - [3, 1]
+ | ...
+box.space._space_sequence:select{}
+ | ---
+ | - - [512, 1, true, 0, '']
+ |   - [513, 2, false, 0, '']
+ | ...
+s:select{}
+ | ---
+ | - - [1]
+ | ...
+_ = s:replace{box.NULL}
+ | ---
+ | ...
+s:select{}
+ | ---
+ | - - [1]
+ |   - [2]
+ | ...
+
+box.space.test2:select{}
+ | ---
+ | - - [1]
+ | ...
+
+box.sequence.seq3:next()
+ | ---
+ | - 2
+ | ...
+
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+test_run:cmd('stop server upgrade')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server upgrade')
+ | ---
+ | - true
+ | ...
diff --git a/test/xlog/gh-4771-upgrade.test.lua b/test/xlog/gh-4771-upgrade.test.lua
new file mode 100644
index 000000000..52334404e
--- /dev/null
+++ b/test/xlog/gh-4771-upgrade.test.lua
@@ -0,0 +1,24 @@
+test_run = require('test_run').new()
+
+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", '..		\
+             'workdir="xlog/upgrade/2.1.3/gh-4771-upgrade-sequence"')
+test_run:cmd('start server upgrade')
+test_run:switch('upgrade')
+
+box.schema.upgrade()
+
+s = box.space.test1
+box.space._sequence:select{}
+box.space._sequence_data:select{}
+box.space._space_sequence:select{}
+s:select{}
+_ = s:replace{box.NULL}
+s:select{}
+
+box.space.test2:select{}
+
+box.sequence.seq3:next()
+
+test_run:switch('default')
+test_run:cmd('stop server upgrade')
+test_run:cmd('delete server upgrade')
diff --git a/test/xlog/suite.cfg b/test/xlog/suite.cfg
deleted file mode 100644
index c33a80ce9..000000000
--- a/test/xlog/suite.cfg
+++ /dev/null
@@ -1,5 +0,0 @@
-{
-    "upgrade.test.lua": {
-        "1.7.7": {"version": "1.7.7"}
-    }
-}
diff --git a/test/xlog/suite.ini b/test/xlog/suite.ini
index 689d2b871..635ec53c1 100644
--- a/test/xlog/suite.ini
+++ b/test/xlog/suite.ini
@@ -2,10 +2,9 @@
 core = tarantool
 description = tarantool write ahead log tests
 script = xlog.lua
-disabled = snap_io_rate.test.lua upgrade.test.lua
+disabled = snap_io_rate.test.lua
 valgrind_disabled =
 release_disabled = errinj.test.lua panic_on_lsn_gap.test.lua panic_on_broken_lsn.test.lua checkpoint_threshold.test.lua
-config = suite.cfg
 use_unix_sockets = True
 use_unix_sockets_iproto = True
 long_run = snap_io_rate.test.lua
diff --git a/test/xlog/upgrade.result b/test/xlog/upgrade.result
deleted file mode 100644
index f64b12d7e..000000000
--- a/test/xlog/upgrade.result
+++ /dev/null
@@ -1,265 +0,0 @@
-test_run = require('test_run').new()
----
-...
-version = test_run:get_cfg('version')
----
-...
-work_dir = "xlog/upgrade/"..version
----
-...
-test_run:cmd('create server upgrade with script="xlog/upgrade.lua", workdir="'..work_dir..'"')
----
-- true
-...
-test_run:cmd("start server upgrade")
----
-- true
-...
-test_run:switch('upgrade')
----
-- true
-...
-test_run:cmd(string.format("push filter '%s' to '<server_uuid>'", box.info.cluster.uuid))
----
-- true
-...
---
--- Upgrade
---
-box.schema.upgrade()
----
-...
---
--- Migrated data
---
-box.space._schema:select()
----
-- - ['cluster', '<server_uuid>']
-  - ['max_id', 513]
-  - ['version', 2, 1, 0]
-...
-box.space._space:select()
----
-- - [257, 1, '_vinyl_deferred_delete', 'blackhole', 0, {'group_id': 1}, [{'name': 'space_id',
-        'type': 'unsigned'}, {'name': 'lsn', 'type': 'unsigned'}, {'name': 'tuple',
-        'type': 'array'}]]
-  - [272, 1, '_schema', 'memtx', 0, {}, [{'type': 'string', 'name': 'key'}]]
-  - [276, 1, '_collation', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {
-        'name': 'name', 'type': 'string'}, {'name': 'owner', 'type': 'unsigned'},
-      {'name': 'type', 'type': 'string'}, {'name': 'locale', 'type': 'string'}, {
-        'name': 'opts', 'type': 'map'}]]
-  - [280, 1, '_space', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
-        'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'engine',
-        'type': 'string'}, {'name': 'field_count', 'type': 'unsigned'}, {'name': 'flags',
-        'type': 'map'}, {'name': 'format', 'type': 'array'}]]
-  - [281, 1, '_vspace', 'sysview', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
-        'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'engine',
-        'type': 'string'}, {'name': 'field_count', 'type': 'unsigned'}, {'name': 'flags',
-        'type': 'map'}, {'name': 'format', 'type': 'array'}]]
-  - [284, 1, '_sequence', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
-        'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'step',
-        'type': 'integer'}, {'name': 'min', 'type': 'integer'}, {'name': 'max', 'type': 'integer'},
-      {'name': 'start', 'type': 'integer'}, {'name': 'cache', 'type': 'integer'},
-      {'name': 'cycle', 'type': 'boolean'}]]
-  - [285, 1, '_sequence_data', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'},
-      {'name': 'value', 'type': 'integer'}]]
-  - [286, 1, '_vsequence', 'sysview', 0, {}, [{'name': 'id', 'type': 'unsigned'},
-      {'name': 'owner', 'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {
-        'name': 'step', 'type': 'integer'}, {'name': 'min', 'type': 'integer'}, {
-        'name': 'max', 'type': 'integer'}, {'name': 'start', 'type': 'integer'}, {
-        'name': 'cache', 'type': 'integer'}, {'name': 'cycle', 'type': 'boolean'}]]
-  - [288, 1, '_index', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'iid',
-        'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'type',
-        'type': 'string'}, {'name': 'opts', 'type': 'map'}, {'name': 'parts', 'type': 'array'}]]
-  - [289, 1, '_vindex', 'sysview', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'iid',
-        'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'type',
-        'type': 'string'}, {'name': 'opts', 'type': 'map'}, {'name': 'parts', 'type': 'array'}]]
-  - [296, 1, '_func', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
-        'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'setuid',
-        'type': 'unsigned'}]]
-  - [297, 1, '_vfunc', 'sysview', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
-        'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'setuid',
-        'type': 'unsigned'}]]
-  - [304, 1, '_user', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
-        'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'type',
-        'type': 'string'}, {'name': 'auth', 'type': 'map'}]]
-  - [305, 1, '_vuser', 'sysview', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
-        'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'type',
-        'type': 'string'}, {'name': 'auth', 'type': 'map'}]]
-  - [312, 1, '_priv', 'memtx', 0, {}, [{'name': 'grantor', 'type': 'unsigned'}, {
-        'name': 'grantee', 'type': 'unsigned'}, {'name': 'object_type', 'type': 'string'},
-      {'name': 'object_id', 'type': 'scalar'}, {'name': 'privilege', 'type': 'unsigned'}]]
-  - [313, 1, '_vpriv', 'sysview', 0, {}, [{'name': 'grantor', 'type': 'unsigned'},
-      {'name': 'grantee', 'type': 'unsigned'}, {'name': 'object_type', 'type': 'string'},
-      {'name': 'object_id', 'type': 'scalar'}, {'name': 'privilege', 'type': 'unsigned'}]]
-  - [320, 1, '_cluster', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'uuid',
-        'type': 'string'}]]
-  - [328, 1, '_trigger', 'memtx', 0, {}, [{'name': 'name', 'type': 'string'}, {'name': 'opts',
-        'type': 'map'}]]
-  - [330, 1, '_truncate', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'count',
-        'type': 'unsigned'}]]
-  - [340, 1, '_space_sequence', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'},
-      {'name': 'sequence_id', 'type': 'unsigned'}, {'name': 'is_generated', 'type': 'boolean'}]]
-  - [348, 1, '_sql_stat1', 'memtx', 0, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
-        'type': 'string'}, {'name': 'stat', 'type': 'string'}]]
-  - [349, 1, '_sql_stat4', 'memtx', 0, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
-        'type': 'string'}, {'name': 'neq', 'type': 'string'}, {'name': 'nlt', 'type': 'string'},
-      {'name': 'ndlt', 'type': 'string'}, {'name': 'sample', 'type': 'scalar'}]]
-  - [512, 1, 'distro', 'memtx', 0, {}, [{'name': 'os', 'type': 'str'}, {'name': 'dist',
-        'type': 'str'}, {'name': 'version', 'type': 'num'}, {'name': 'time', 'type': 'num'}]]
-  - [513, 1, 'temporary', 'memtx', 0, {'temporary': true}, []]
-...
-box.space._index:select()
----
-- - [272, 0, 'primary', 'tree', {'unique': true}, [[0, 'string']]]
-  - [276, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
-  - [276, 1, 'name', 'tree', {'unique': true}, [[1, 'string']]]
-  - [280, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
-  - [280, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
-  - [280, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
-  - [281, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
-  - [281, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
-  - [281, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
-  - [284, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
-  - [284, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
-  - [284, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
-  - [285, 0, 'primary', 'hash', {'unique': true}, [[0, 'unsigned']]]
-  - [286, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
-  - [286, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
-  - [286, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
-  - [288, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned'], [1, 'unsigned']]]
-  - [288, 2, 'name', 'tree', {'unique': true}, [[0, 'unsigned'], [2, 'string']]]
-  - [289, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned'], [1, 'unsigned']]]
-  - [289, 2, 'name', 'tree', {'unique': true}, [[0, 'unsigned'], [2, 'string']]]
-  - [296, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
-  - [296, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
-  - [296, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
-  - [297, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
-  - [297, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
-  - [297, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
-  - [304, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
-  - [304, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
-  - [304, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
-  - [305, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
-  - [305, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
-  - [305, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
-  - [312, 0, 'primary', 'tree', {'unique': true}, [[1, 'unsigned'], [2, 'string'],
-      [3, 'scalar']]]
-  - [312, 1, 'owner', 'tree', {'unique': false}, [[0, 'unsigned']]]
-  - [312, 2, 'object', 'tree', {'unique': false}, [[2, 'string'], [3, 'scalar']]]
-  - [313, 0, 'primary', 'tree', {'unique': true}, [[1, 'unsigned'], [2, 'string'],
-      [3, 'scalar']]]
-  - [313, 1, 'owner', 'tree', {'unique': false}, [[0, 'unsigned']]]
-  - [313, 2, 'object', 'tree', {'unique': false}, [[2, 'string'], [3, 'scalar']]]
-  - [320, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
-  - [320, 1, 'uuid', 'tree', {'unique': true}, [[1, 'string']]]
-  - [328, 0, 'primary', 'tree', {'unique': true}, [[0, 'string']]]
-  - [330, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
-  - [340, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
-  - [340, 1, 'sequence', 'tree', {'unique': false}, [[1, 'unsigned']]]
-  - [348, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'string']]]
-  - [349, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'string'], [
-        5, 'scalar']]]
-  - [512, 0, 'primary', 'hash', {'unique': true}, [[0, 'string'], [1, 'string'], [
-        2, 'unsigned']]]
-  - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
-  - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
-...
-box.space._user:select()
----
-- - [0, 1, 'guest', 'user', {'chap-sha1': 'vhvewKp0tNyweZQ+cFKAlsyphfg='}]
-  - [1, 1, 'admin', 'user', {}]
-  - [2, 1, 'public', 'role', {}]
-  - [3, 1, 'replication', 'role', {}]
-  - [31, 1, 'super', 'role', {}]
-  - [32, 1, 'someuser', 'user', {'chap-sha1': '2qvbQIHM4zMWhAmm2xGeGNjqoHM='}]
-  - [33, 1, 'somerole', 'role', {}]
-...
-box.space._func:select()
----
-- - [1, 1, 'box.schema.user.info', 1, 'LUA']
-  - [2, 32, 'somefunc', 1, 'LUA']
-  - [3, 1, 'someotherfunc', 0, 'LUA']
-...
-box.space._collation:select()
----
-- - [1, 'unicode', 1, 'ICU', '', {}]
-  - [2, 'unicode_ci', 1, 'ICU', '', {'strength': 'primary'}]
-...
-box.space._priv:select()
----
-- - [1, 0, 'role', 2, 4]
-  - [1, 0, 'universe', 0, 24]
-  - [1, 1, 'universe', 0, 4294967295]
-  - [1, 2, 'function', 1, 4]
-  - [1, 2, 'function', 2, 4]
-  - [1, 2, 'space', 276, 2]
-  - [1, 2, 'space', 281, 1]
-  - [1, 2, 'space', 286, 1]
-  - [1, 2, 'space', 289, 1]
-  - [1, 2, 'space', 297, 1]
-  - [1, 2, 'space', 305, 1]
-  - [1, 2, 'space', 313, 1]
-  - [1, 2, 'space', 330, 2]
-  - [1, 3, 'space', 320, 2]
-  - [1, 3, 'universe', 0, 1]
-  - [1, 31, 'universe', 0, 4294967295]
-  - [1, 32, 'function', 3, 4]
-  - [1, 32, 'role', 2, 4]
-  - [1, 32, 'role', 33, 4]
-  - [1, 32, 'space', 513, 3]
-  - [1, 32, 'universe', 0, 24]
-  - [1, 33, 'space', 512, 3]
-...
-box.space._vspace ~= nil
----
-- true
-...
-box.space._vindex ~= nil
----
-- true
-...
-box.space._vuser ~= nil
----
-- true
-...
-box.space._vpriv ~= nil
----
-- true
-...
--- a test space
-r = box.space.distro:select()
----
-...
-_ = table.sort(r, function(left, right) return tostring(left) < tostring(right) end)
----
-...
-r
----
-- - ['debian', 'etch', 40, 1176019200]
-  - ['debian', 'jessie', 80, 1430038800]
-  - ['debian', 'lenny', 50, 1234602000]
-  - ['debian', 'sarge', 31, 1118044800]
-  - ['debian', 'squeeze', 60, 1296896400]
-  - ['debian', 'wheezy', 70, 1367654400]
-  - ['debian', 'woody', 30, 1027065600]
-  - ['ubuntu', 'precise', 1510, 1335427200]
-  - ['ubuntu', 'trusty', 1404, 1397721600]
-  - ['ubuntu', 'vivid', 1504, 1429779600]
-  - ['ubuntu', 'wily', 1510, 1445504400]
-...
-test_run:cmd("clear filter")
----
-- true
-...
-test_run:switch('default')
----
-- true
-...
-test_run:cmd('stop server upgrade')
----
-- true
-...
-test_run = nil
----
-...
diff --git a/test/xlog/upgrade.test.lua b/test/xlog/upgrade.test.lua
deleted file mode 100644
index 0be2d34e9..000000000
--- a/test/xlog/upgrade.test.lua
+++ /dev/null
@@ -1,46 +0,0 @@
-test_run = require('test_run').new()
-
-version = test_run:get_cfg('version')
-work_dir = "xlog/upgrade/"..version
-
-test_run:cmd('create server upgrade with script="xlog/upgrade.lua", workdir="'..work_dir..'"')
-test_run:cmd("start server upgrade")
-
-test_run:switch('upgrade')
-
-test_run:cmd(string.format("push filter '%s' to '<server_uuid>'", box.info.cluster.uuid))
-
---
--- Upgrade
---
-
-box.schema.upgrade()
-
---
--- Migrated data
---
-
-box.space._schema:select()
-box.space._space:select()
-box.space._index:select()
-box.space._user:select()
-box.space._func:select()
-box.space._collation:select()
-box.space._priv:select()
-
-box.space._vspace ~= nil
-box.space._vindex ~= nil
-box.space._vuser ~= nil
-box.space._vpriv ~= nil
-
--- a test space
-r = box.space.distro:select()
-_ = table.sort(r, function(left, right) return tostring(left) < tostring(right) end)
-r
-
-test_run:cmd("clear filter")
-
-test_run:switch('default')
-test_run:cmd('stop server upgrade')
-
-test_run = nil
diff --git a/test/xlog/upgrade/2.1.3/gh-4771-upgrade-sequence/00000000000000000014.snap b/test/xlog/upgrade/2.1.3/gh-4771-upgrade-sequence/00000000000000000014.snap
new file mode 100644
index 0000000000000000000000000000000000000000..c0d2ef404c4e54377230a3134394dbc7abb5f730
GIT binary patch
literal 4622
zcmV+p67lU*PC-x#FfK7O3RY!ub7^mGIv_GGF)lMLI4x%~WHB~kIW{;p3Q2BrbYX5|
zWjY`?V>dKpWHvP|I5RmgEi^DQGA%hZVKyx{Vlg&1V>UQsFgY~}RzqxWV{1Afdoem7
zF*JP&3e~y`y3G-h0L~EUMG>U{00000D77#B08mAY0D7KqBS#R3Sla*q002M{;9%ex
zAG}a+-T!Zg3?|=51CYO~kcd*IWR#gCX}NN9<xJcl`Sg28z6)|bW|^t{WpeN9es2cF
z|1qIN0x<$E0uTbd4LmqcF&&&|7!J;})nJ3w%s|X)Vi0CER}+i|;G3olXm2jSV8=uy
z?B$y;V9L@<Q(0PYLD^RDCGB<bLhDgrL8?n#k4pt$*Oo52)IxzS-RQ0?Ty%+Li!LY7
zqDx)2SF)6)6)a_0WrDJ7sa*Mgik1JFs{G$BQ32^lFeL&e68G<O8@##xn#BEwq0e-r
zEI}Ya&JRS8vjb9iu0g=F(gQs69L0g0lWI95DdpTki6Z(Qm?Zih4N3H*PV~JQ6p+pl
zNTbsLt%+&Y#6;)G4D2#GqIEIQ0LsU34XsoOGs%T=F6PZoH4Jl?lP8#5zzCB?3}6zI
zL{4rBEO5IeP;|Q?K;)Y!;Qav!cy9m#cx+`&@Ypgr0FPF9Y?l}Sat+C7TtZB+(te}D
z6|}?`FyuV}Lf!)f?Vken9)N(o1pvxo06$su86b;UR3ZQlmm~lj4v7yA*Q3PyBT2kB
zg2ekGJ-rh-()$o2y#r9hW)nihW(zXJX2UHI0KgwY$j!&>wP-$ql-O$K!xD#W4<NUW
zw-$M^A2!`pEIo#7U_N57m(;Ce{d@jd=Qnrm-)EIs1BOZ#-OVH)^<J@Vj~MJFMP^Ny
zFHBvsOyS+U`M!Ep$$Vll*h?yQVVi5;GmC{L275_e*pJj<6t?kh4EB;DF*rjKMKNj2
zpwX<BMWYO*2F0ib1=oy_l5j4?5jVlp^Y?2yV&!B#_w~BdV`l4%rI-)7DVUV#0ehJV
zR&5txoS%y?&cB5h=Rdy|UHqQ~7yo0i#sBRUTHtR*7Wh|z1^)QsxdN0!g%vb8Q&hqF
zH)OVTUgjQ#td}DN9`B=|!h0yD@a+>)fLu;QLE~xy3apR0gxzy7@kIAaIMMx`Cc5wb
z$HbHOE%D@)xbx(nd=fwJk??tcgc9Bxk%ad}AmKd`NBDLKBfvR!ilEJ@L&T^_-=>e6
z3`6bb2r-2ALD<nAh&tM9|AP?O`w&F-Jphq?_B{N6JslsgpVI^O*mv>3yBs>a$C<-B
z?{K<Obq6`AszaPqO;s*9Sj}MvtiI6!t5|iz)p;+6&f7PgcizU0jc@gAoK0)vHm=ns
zYm57h#l~-)@A0(uE^F^Ati5$Saa--gjl-3=#l1%NZ?O8`1|0scVNIvuhTdnep?4W-
zNY&7grZLQrrfFnAZB=ThVnA)E4NV%BJyXTzs91Ii&Hpf*X<x{tR;>$DJ8c1mowD%4
zPItP33o5ig>!rYg4^4Gw+CyOl@B|fnXlp_WKC~?X1(1Xjd_5@>d>qxGW_%@pWqc%X
zPR1wYL&uo$fg_ahp=c`t8Q(@X8p4<q1!05~#fgxjDS8V5MWaAK(H07aTA?j_!%k}y
zcKT4*2|Ib#>%MyZ^u6|4^`_?C)ceWQrnafKGv&7v-M900I@#7!r|Ws5>$#q$(|MNq
zJo9;;Eoq+TSe~bt&lZz8e=z18iaC3L!CnU+(0Uw%Q25kA;WIt^b%wzCS4`EjU&Gx!
zYJz6mYsrSZTXewsvqv-TwG4Nc$bc#~#JVzB$Ldb&s-iNvb;!F+n5bbxHg{;r$7I;M
zLPHzplyfm1Z}}YYxceP{zl*|K+jKpf`EQM-=I^i6?7Ti@A8OqoA*ifbfAd&uh;@O2
zKDKpU@=TnTyT4aHhGd#`xwW;qGcQfZyFUe}*rxMe<yUy*DNnuqo;MWVSygSWKTcCV
zhHS160d{gK;xt`%==D}!Q_Sve8|$QQ8P%Cv_wRXTa%5gn-<EAVY|H29L#*3#f4FzN
z%c;8l-S(-D%-hs{uUT2W{VP(3dBm#E$~L^O%$&KT4%L-+Gpo!ir~E`moQhic=QNWc
znS0M=Cd2f$H5<mc_x{qzetvugP$Iy3;*tb_#HU9|MtT4KND>f^5hOG?MSApYYZVTW
zBVgAMBeX66iu7nRkS3a4LWqEIf()Snxxf26ac=O5WNI;t^Fzpq^AY64^#F3>PF(B<
z2)jQYw0nO&ZnxL{#lSqiDw)sy(em(B$$ah)3aZ89b$?**jy$xU9eH3V9fs=Ffd^;l
zxP!BE*uh!O%29`A<Df&caLl1u&AuT=U)_kKuWi85*S@mx#=5TI#=5G}#@eRA2FQiR
z8Z@pm)bOoV=E`M88vU1nM*n1-(SQGAnDM_DW&9ro8UMFnjPd^$V*KBF5k_}Pc41?t
zWG~nVbzoP7`-1(>$eKbxsvVI^Nn@0xnHa?=2_w~vXy6ePqoW$AWQ3GNl2D?IQJUbn
zIoT2jOK|tL6pMmsfvT~ywdl+IP`^M=FrWL|!PL9gG7HA@<jIrg&fPz+?0aVQUbAdH
zV6RIhQ%ZYQSHHVU&{3yq?prtJlvCcXMxBbQ%DXTs+0p%NZ6opS-cyQMp6bn~Ok1vu
zPP~Z8P`57sNZkUJ*^}*IhEH##;&aq0)3aZ%zR#<<yDuu)k;w#xi|+Dr>nA<?^~sYc
zF79wIm5ivou#Bo!uNREG<d`GHtc#mI&=OuO9$@L<E^)B5K}}38OihCv52lpCO9_Km
z?u-(cS<~!-*_R_PM;07aP}HS}f~g`dL|cfoOc7;~VadrRU~(nL#L7^WAu2;tPLEF^
zrl6%D34$U3k^%$)&;u~oj{`S1Rp<WHew6Nw;*<2sP1(uolin+@OwRpZy?XAiO2!1~
z=43+mmy-$IUo0mRqJgm)?J4`K3sWNtI;!)!dB!Yp?^E`n*1c+EK@HIE6BuUS=fArz
zn9)|sM>d#L*Kl*K&9%<)W1H7oJ9n&Vk*PjMHGgZ|zu$Ze(K75^k^wS*-1E+6=yjW|
zGj48^lU*)<JnpssQ0r=0;EZYN{==+`rD)BCS$Bzu)m^PAVCGb_x~p0^#iQqGg@4RW
zBHzlH|Bbte#p<qxyQ`D{Rfce=<HMJlLL(yp000mO07Dx91P}%0mRXVk69B-0!hzwU
zF*p_r#2ALma*8rF00SUkL4YCv&<vzC2j4G7O~qi-8(#Wk8HjN2jWmX03p20O)F@19
zdK4x-Ju)GtXh?@tZ@n4^_&2_2T_I{7*O(t1r~=zGoYVZIQ28F{x#lxPw0Hqse-LIF
zDwBPtOQS+Z2#6I%3EqqcPFPicW*r|6`!j8l20PzB5DmdEx@(qgcsit?CKS=prS6x9
z8ZacRKmGKQWK}j1P*$O@XV(Uac4=IyICYnalikjoegQEGu!Y(N^z5x+rkCR|r+zDP
z`*zF-!oLIA)l6Mh@J-PcU$r}0?4ZSjyIO3Z#DqVu6>kTrV~W=IwesE*5HYoQsRet4
zQ&8h2)%+v3I2sk7UJAZyb%7Rcwh9D;_&&KhSI$9Pr(gpGF7(xE2Q4n%<_#JiKYmkV
zg<1=^;F}L%r!gI<aNYSXO|UNq;2JAuFh@QpT`8;Ig(-&|ovw_V&I?0FM`O6nsM66l
zH#z>ONahO7pYCQ<o)D|Uf>yU4C4=P<D4tdA7xM;hvs;<-KaVU#*R^VS!-RD@GmWg7
zs<gsaCwnQktmSo6c$_9KyU-;#q8uJ_Db71=Y*l0cUU#t)A<CmIlz+5!)B^traF;d4
z6p7Jg3Z!gCL%2?BNR63oZ{#GksTzEKGW^0J#f_o)uIazxvB$hyz`JAGMm=~I7JCQ%
zCWp_rZznRP8}Wu1ru)0{8~+g61U?@vwb)cgCcc4|miiMpOQ4B9#7E7Z4xAeMjOe&H
zKXbhF6Ac?+=veAtVGF@Ku=y=}wXys>%JVjKzeeCb*nR4-JAZLi1z&@%Prq7V1KDq0
zDki)_pU5>trr;pkX?M2xmusdbeDq^(u^(ON(TnvI{7ys#P5ko(_Rlya0itXl`H>g8
zebqNb+tm%l<VW#dK}F*MDGX++rTK0=fW3A!F-7YwDk7tB9kW&)DCX>P@zXkX6{?+z
z+n?r=Uy*`m=y(CXSsAlAy3|9pbYrpeaQ8PB67b~aL~!H1<~Y~*|6Fv-ZVcsget6B%
z8HE}=3+%MUJ)Yag;6p<NX4DPY2!hM;zU?oeu+}{8+bka@%8%Sd4>DGb@5~#P@xI7_
zoP6Ihm)95rq!T5QGNdHLFJ`*U?oHZ40S<c?JgiBu=<iFn`6T3WtH3nhvTgzEayQDb
z1gWdYIM{aa&T6Xy3A6&yJ2hs&LV$<YRP+*r4(W^mR?-3VozU9p?zDJFx?3hRD55hl
zXeLUihI~+3<m0B2o`?-~upKX`Ex^7scR?aQB*z=bkJ1eW#vC$uV!YNUjWS~b6rYeG
zllP#83DqbAA!)=sS4OhQK$jyJykCtPTr#m)!GQ}~KpE@>c82gLYQr{X&>LxBH3Vi9
zxQs}kQcc*?rYN(#R{3+%nR=Gn7GnP$J;`>KP%962B)lnUt^sZqKWShX3FZ}%tkPT3
z&O8r9Ks5!{AfOv7NR~MMO0^)&-dqh<HD2J~bS&x@O_)<hdGQtW3BTK$uw>pD%Sjd=
zGP<GdjY{ejXydrk6T+&))Sx?(oZ;LNG1-N~LWX5VX^hkvLsF2o1F|!F1lEwyjFluL
zjfm%pNKS%G8Zpn6k-W2o_6Up-uT}R3xcAAP=k;;R#G8|bG(U+eL-T1aKs377GyvUM
z52S!<3an9J8&imgd>|cmLtu>p+n7Q`<OAuj8v<(-*v1qhA|FVH-4Ix#z&5535&1wm
z?1sP^1-3DTh{y-hVK)TUD6owwL_|K24!a?+MuBZiAtLgDbl44nH41EF3K7vGPJSg#
zd3a<KF9{m}u?`iX%cfDWSr;@Vom^99Or4By7^q}6oP<>MG{csBZKA0+X@)eCU_66_
zj>OWDgO~<15;+McCqob^k^_Z>FmaObf`xIX00g-P6pfx~GhEsezlH?QZ&uk)nLtSf
zITs1W8Pgb&X9kHt%^@w*%HIZumVY43MlmkxJGA9X%H>sz`rQ2V0xB3us}16uaSZ<u
z4$o`ks_+%ATsj#Z5k{oEQ5TI#HA$f4ivnhV37mlgia?KYdecu~8N`p2A49Fn3%y#0
zKhVcZ=O`os*A}Yc6eISnh?ZCsn?{~FIT+t#MYP0QP7!a~`6bakrF&uPZY+)pxDkL)
zMHVcdeTER+8G^VzD&T<i*fF-`j!=nnQ89;%%Z{-ncZ5oui;6j9Ty~5txg%8KTvW^<
z<AD&Mny|}`D`pPGPR&YL7mxfpzj6(~sJQkes@$*K;zcqFbxdLp8KKt($56|`7_Lm|
zhgVLyL?nLPocsw8GB%ZrJ`-h9w;f6sLbW0ossGo>?@C$cD>Ef8kr7;y*gotp-~iQ}
z-Fq3E7AriRwjh6tQ*V?VBMi(sJ6UwURVIoNYU@OF*PSOuel;JAIX_vqs$L;p7Rt7I
zYYfZ9UI@syg5W%|s0ToREJ{v1>@$v%WH4A<`6E-a``a8Aa&PUtz|xz4KIu)h^KsBV
zJ|%KgO|@i&j`sV*jeMy<otyo(*PbzDavVjB4;5Lkd<YS=F>}pEg?&Kbc^U#f-mIvv
zW{=^De6hPydf0{(r|AQ-dvfDoPK6!<J0A|_O~fIp*hK`(1T|cDKFqq?{uny%)4e$e
zK^63j6j-Le$+Y&<H}*)*BZ>2%q78=ArJR;&ru~>{Usv%()>!j1*2GV))3AiI_&|8N
zFySh!{)B?=N}9Ot^gp{U12F?sX{cWZ+t2A9J8YZH`SX$sBCyewuS7S;;f%jjFMk#7
zjE6b?(p4jHqePWx2sOpdIGbU%f8ESk_xtN6j9zJb7xDD+&bpto?)TSC7`>9L$;P(t
z7o!FYu%1oKE<4fDcNNYmhB^D(`>!K(K;aj=09>n|-^HkB$+wRH^H;)tk`Q9a2h|X*
E?GdQ4X#fBK

literal 0
HcmV?d00001

diff --git a/test/xlog/upgrade/2.1.3/gh-4771-upgrade-sequence/fill.lua b/test/xlog/upgrade/2.1.3/gh-4771-upgrade-sequence/fill.lua
new file mode 100644
index 000000000..b159f2b67
--- /dev/null
+++ b/test/xlog/upgrade/2.1.3/gh-4771-upgrade-sequence/fill.lua
@@ -0,0 +1,14 @@
+box.cfg{}
+s1 = box.schema.create_space('test1')
+pk = s1:create_index('pk', {sequence = true})
+s1:replace{box.NULL}
+
+seq2 = box.schema.sequence.create('seq2')
+s2 = box.schema.create_space('test2')
+pk = s2:create_index('pk', {sequence = 'seq2'})
+s2:replace{box.NULL}
+
+seq3 = box.schema.sequence.create('seq3')
+seq3:next()
+
+box.snapshot()
-- 
2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH 1/3] upgrade: add missing sys triggers off and erasure

[Nikita Pettik]

On 17 Feb 21:57, Vladislav Shpilevoy wrote:

LGTM

> +local function foreach_system_space(cb)
> +    local max = box.schema.SYSTEM_ID_MAX
> +    for id, space in pairs(box.space) do

Nit: I'd add brief comment explaining that here we are interested
only in 'native' system spaces skipping sysviews etc.

> +        if type(id) == 'number' and
> +           (space.engine == 'memtx' or space.engine == 'vinyl') then
> +            if id > max then
> +                break
> +            end
> +            cb(space)
> +        end
> +    end
> +end
> +
>  local function set_system_triggers(val)
> -    box.space._space:run_triggers(val)
> -    box.space._index:run_triggers(val)
> -    box.space._user:run_triggers(val)
> -    box.space._func:run_triggers(val)
> -    box.space._priv:run_triggers(val)
> -    box.space._trigger:run_triggers(val)
> -    box.space._collation:run_triggers(val)
> -    box.space._schema:run_triggers(val)
> -    box.space._cluster:run_triggers(val)
> -    box.space._fk_constraint:run_triggers(val)
> -    box.space._ck_constraint:run_triggers(val)
> +    foreach_system_space(function(s) s:run_triggers(val) end)
>  end
>  

Re: [Tarantool-patches] [PATCH 2/3] box: forbid to update/replace _space_sequence

[Nikita Pettik]

On 17 Feb 21:57, Vladislav Shpilevoy wrote:
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index 64af71c61..50121c79c 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -4713,9 +4713,13 @@ on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event)
>  		return -1;
>  	}

LGTM

Nit: as always I'd only add brief comment like:

/* UPDATE operation is considered to meaningless for _space_sequence
   since ...

*/
  
> +	if ((stmt->new_tuple != NULL) == (stmt->old_tuple != NULL)) {
> +		diag_set(ClientError, ER_UNSUPPORTED,
> +			 "space \"_space_sequence\"", "update");
> +		return -1;
> +	}
> +
>  	enum priv_type priv_type = stmt->new_tuple ? PRIV_C : PRIV_D;
> -	if (stmt->new_tuple && stmt->old_tuple)
> -		priv_type = PRIV_A;
>  
>  	/* Check we have the correct access type on the sequence.  * */
>  	if (is_generated || !stmt->new_tuple) {

Re: [Tarantool-patches] [PATCH 3/3] upgrade: fix generated sequence upgrade from 2.1

[Nikita Pettik]

On 17 Feb 21:57, Vladislav Shpilevoy wrote:
> The bug was in an attempt to update a record in _space_sequence
> in-place, to add field path and number. This was not properly
> supported by the system space's trigger, and was banned in the
> previous patch of this series.
> 
> But delete + tuple update + insert work fine. The patch uses them.
> 
> To test it the old disabled and heavily outdated
> xlog/upgrade.test.lua was replaced with a smaller analogue, which
> is supposed to be created separately for each upgrade bug.
> According to the new policy of creating test files.
> 
> The patch tries to make it easy to add new upgrade tests and
> snapshots. A new test should consist of fill.lua script to
> populate spaces, snapshot, needed xlogs, and a .test.lua file.
> Fill script and binaries should be in the same folder as test file
> name, which is located in version folder. Like this:

I'm okay with this approach but could you document it somewhere
(e.g. on our wiki github page)?

>  xlog/
>  |
>  + <test_name>.test.lua
>  |
>  +- upgrade/
>     |
>     +- <version>/
>     |   |
>     |   +-<test_name>/
>     |     |
>     |     +- fill.lua
>     |     +- *.snap
>     |     +- *.xlog
> 
> Version is supposed to say explicitly what a version files in
> there have.

Patch itself is LGTM.

Re: [Tarantool-patches] [PATCH 3/3] upgrade: fix generated sequence upgrade from 2.1

[Vladislav Shpilevoy]

Thanks for the review!

On 21/02/2020 16:42, Nikita Pettik wrote:
> On 17 Feb 21:57, Vladislav Shpilevoy wrote:
>> The bug was in an attempt to update a record in _space_sequence
>> in-place, to add field path and number. This was not properly
>> supported by the system space's trigger, and was banned in the
>> previous patch of this series.
>>
>> But delete + tuple update + insert work fine. The patch uses them.
>>
>> To test it the old disabled and heavily outdated
>> xlog/upgrade.test.lua was replaced with a smaller analogue, which
>> is supposed to be created separately for each upgrade bug.
>> According to the new policy of creating test files.
>>
>> The patch tries to make it easy to add new upgrade tests and
>> snapshots. A new test should consist of fill.lua script to
>> populate spaces, snapshot, needed xlogs, and a .test.lua file.
>> Fill script and binaries should be in the same folder as test file
>> name, which is located in version folder. Like this:
> 
> I'm okay with this approach but could you document it somewhere
> (e.g. on our wiki github page)?

Ok, added a file with instructions to xlog/upgrade. I decided not to
add it to wiki, because anyway no one reads it, when just wants to add
a test. There is a higher chance, that this file will be noticed.

diff --git a/test/xlog/upgrade/how_to_add_new_test.md b/test/xlog/upgrade/how_to_add_new_test.md
new file mode 100644
index 000000000..3fa3c2d44
--- /dev/null
+++ b/test/xlog/upgrade/how_to_add_new_test.md
@@ -0,0 +1,36 @@
+## How to add a new upgrade test
+
+Firstly, for a new test should be prepared a `fill.lua` file. Here
+should be code to populate needed spaces, create xlogs, snapshots.
+
+`Fill.lua` should be run in an old Tarantool version, upgrade from
+which is going to be tested. This will produce `.xlog` and `.snap`
+files. They should be saved into
+`xlog/upgrade/<version>/<test_name>/` folder. Snapshot is usually
+enough. Version is the old Tarantool version. Test name is a name
+of test file, which will test recovery from these xlogs. It should
+obey to the same rules as other test file names.
+
+Secondly, it is necessary to add a `<test_name>.test.lua` file to
+`xlog/` folder, to all the other tests. It should start a new
+instance of Tarantool with workdir set to
+`xlog/upgrade/<version>/<test_name>/`. Then this file should
+ensure, that `box.upgrade()` works fine, and all data is recovered
+correctly.
+
+The following directory structure should be in the result:
+```
+ xlog/
+ |
+ +- <test_name>.test.lua
+ |
+ +- upgrade/
+    |
+    +- <version>/
+        |
+        +- <test_name>/
+           |
+           +- fill.lua
+           +- *.snap
+           +- *.xlog
+```

Re: [Tarantool-patches] [PATCH 2/3] box: forbid to update/replace _space_sequence

[Vladislav Shpilevoy]

Thanks for the review!

On 21/02/2020 16:29, Nikita Pettik wrote:
> On 17 Feb 21:57, Vladislav Shpilevoy wrote:
>> diff --git a/src/box/alter.cc b/src/box/alter.cc
>> index 64af71c61..50121c79c 100644
>> --- a/src/box/alter.cc
>> +++ b/src/box/alter.cc
>> @@ -4713,9 +4713,13 @@ on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event)
>>  		return -1;
>>  	}
> 
> LGTM
> 
> Nit: as always I'd only add brief comment like:

Done. New hunk is below. Also I changed

    (stmt->new_tuple != NULL) == (stmt->old_tuple != NULL)

to

    stmt->new_tuple != NULL && stmt->old_tuple != NULL

Because both of the tuples can't be NULL anyway.

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 64af71c61..d73679fb8 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -4712,10 +4712,17 @@ on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event)
 		diag_set(ClientError, ER_NO_SUCH_SEQUENCE, int2str(sequence_id));
 		return -1;
 	}
-
+	if (stmt->new_tuple != NULL && stmt->old_tuple != NULL) {
+		/*
+		 * Makes no sense to support update, it would
+		 * complicate the code, and won't simplify
+		 * anything else.
+		 */
+		diag_set(ClientError, ER_UNSUPPORTED,
+			 "space \"_space_sequence\"", "update");
+		return -1;
+	}
 	enum priv_type priv_type = stmt->new_tuple ? PRIV_C : PRIV_D;
-	if (stmt->new_tuple && stmt->old_tuple)
-		priv_type = PRIV_A;

Re: [Tarantool-patches] [PATCH 1/3] upgrade: add missing sys triggers off and erasure

[Vladislav Shpilevoy]

Hi! Thanks for the review!

On 21/02/2020 16:20, Nikita Pettik wrote:
> On 17 Feb 21:57, Vladislav Shpilevoy wrote:
> 
> LGTM
> 
>> +local function foreach_system_space(cb)
>> +    local max = box.schema.SYSTEM_ID_MAX
>> +    for id, space in pairs(box.space) do
> 
> Nit: I'd add brief comment explaining that here we are interested
> only in 'native' system spaces skipping sysviews etc.

Done. Also I decided not to break, when see a space id > SYSTEM_ID_MAX,
because box.space is not an array. And it is not safe to assume, that
all numeric indexes here are in ascending order.

diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index bd28c1001..2f029d8c0 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -66,11 +66,18 @@ end
 local function foreach_system_space(cb)
     local max = box.schema.SYSTEM_ID_MAX
     for id, space in pairs(box.space) do
-        if type(id) == 'number' and
+        -- Check for number, because box.space contains each space
+        -- twice - by ID and by name. Here IDs are selected.
+        -- Engine is checked to be a 'native' space, because other
+        -- engines does not support DML, and does not have
+        -- triggers to turn off/on. These are 'service',
+        -- 'blackhole', and more may be added.
+        -- When id > max system id is met, break is not done,
+        -- because box.space is not an array, and it is not safe
+        -- to assume all its numeric indexes are returned in
+        -- ascending order.
+        if type(id) == 'number' and id <= max and
            (space.engine == 'memtx' or space.engine == 'vinyl') then
-            if id > max then
-                break
-            end
             cb(space)
         end
     end

Re: [Tarantool-patches] [PATCH 0/3] Fix upgrade from 2.1 sequence

[Kirill Yukhin]

Hello,

On 17 фев 21:57, Vladislav Shpilevoy wrote:
> The patchset fixes inability to upgrade from 2.1 if there is an
> automatically generated sequence. The problem was that upgrade
> tried to update a tuple in _space_sequence in-place, calling
> space:update() method. But it was not properly supported for
> _space_sequence. The patchset turns it into delete + insert, and
> bans updates for this space.
> 
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4771-2.2-upgrade
> Issue: https://github.com/tarantool/tarantool/issues/4771

I've checked your patch into 2.2, 2.3 and master.

--
Regards, Kirill Yukhin