From e52910381474a51aafa288f87836098da16cefbe Mon Sep 17 00:00:00 2001 From: Vladimir Davydov <vdavydov@tarantool.org> Date: Wed, 8 Nov 2023 16:48:26 +0300 Subject: [PATCH] box: decrement BOX_SPACE_MAX Currently, BOX_SPACE_MAX equals BOX_ID_NIL, which is used as an error indicator in the box C API. As a result, if box_space_id_by_name() returns BOX_ID_NIL, it's impossible to figure out whether there's no space with the give name or the space exists and has the id equal to BOX_SPACE_MAX. Let's decrement BOX_SPACE_MAX to fix this issue. Since this may break recovery, let's also introduce the new compatibility module option `box_space_max` to allow the user revert to the old behavior. Closes #9118 @TarantoolBot document Title: Document `compat.box_space_max` Please create an entry for the new compatibility module option at https://tarantool.io/compat/box_space_max. `compat.box_space_max` sets the max space id (`box.schema.SPACE_MAX`). The old limit is 2147483647. The new limit is 2147483646. The limit was decremented because the old value is used as an error indicator in the box C API (it equals `BOX_ID_NIL`). --- .../unreleased/gh-9118-box-space-max.md | 6 +++ src/box/schema_def.c | 8 +++ src/box/schema_def.h | 4 +- src/lua/compat.lua | 14 ++++++ .../fully-temporary_spaces_test.lua | 3 +- ...ace_with_explicit_and_implicit_id_test.lua | 4 +- test/box-luatest/gh_9118_space_max_test.lua | 49 +++++++++++++++++++ test/box/alter_limits.result | 2 +- 8 files changed, 84 insertions(+), 6 deletions(-) create mode 100644 changelogs/unreleased/gh-9118-box-space-max.md create mode 100644 test/box-luatest/gh_9118_space_max_test.lua diff --git a/changelogs/unreleased/gh-9118-box-space-max.md b/changelogs/unreleased/gh-9118-box-space-max.md new file mode 100644 index 0000000000..2e702ff28d --- /dev/null +++ b/changelogs/unreleased/gh-9118-box-space-max.md @@ -0,0 +1,6 @@ +## bugfix/core + +* Decremented the max space id (`box.schema.SPACE_MAX`). Now, the max space id + equals 2147483646. The limit was decremented because the old value is used as + an error indicator in the box C API. It's still possible to revert to the old + behavior with the compatibility module option `box_space_max` (gh-9118). diff --git a/src/box/schema_def.c b/src/box/schema_def.c index 98a1d207a8..211f0f9db7 100644 --- a/src/box/schema_def.c +++ b/src/box/schema_def.c @@ -29,6 +29,14 @@ * SUCH DAMAGE. */ #include "schema_def.h" +#include "tweaks.h" + +/** + * Don't let the user create a space with id = BOX_ID_NIL because + * it's used as an error indicator in the box C API. + */ +uint64_t BOX_SPACE_MAX = BOX_ID_NIL - 1; +TWEAK_UINT(BOX_SPACE_MAX); const char *sql_storage_engine_strs[] = { [SQL_STORAGE_ENGINE_MEMTX] = "memtx", diff --git a/src/box/schema_def.h b/src/box/schema_def.h index 1029ccb1a2..2599b631c5 100644 --- a/src/box/schema_def.h +++ b/src/box/schema_def.h @@ -39,7 +39,6 @@ extern "C" { enum { BOX_ENGINE_MAX = 3, /* + 1 to the actual number of engines */ - BOX_SPACE_MAX = INT32_MAX, BOX_FUNCTION_MAX = 32000, BOX_INDEX_MAX = 128, BOX_NAME_MAX = 65000, @@ -131,6 +130,9 @@ enum { }; /** \endcond public */ +/** Max possible space id. */ +extern uint64_t BOX_SPACE_MAX; + /** _space fields. */ enum { BOX_SPACE_FIELD_ID = 0, diff --git a/src/lua/compat.lua b/src/lua/compat.lua index e739e2a398..e4c1c60013 100644 --- a/src/lua/compat.lua +++ b/src/lua/compat.lua @@ -103,6 +103,14 @@ IPROTO_FEATURE_CALL_ARG_TUPLE_EXTENSION feature bits. https://tarantool.io/compat/box_tuple_extension ]] +local BOX_SPACE_MAX_BRIEF = [[ +Controls the max space id (box.schema.SPACE_MAX). The old limit is 2147483647. +The new limit is 2147483646. The limit was decremented because the old value is +used as an error indicator in the box C API. + +https://tarantool.io/compat/box_space_max +]] + -- Returns an action callback that toggles a tweak. local function tweak_action(tweak_name, old_tweak_value, new_tweak_value) return function(is_new) @@ -196,6 +204,12 @@ local options = { run_action_now = true, action = tweak_action('box_tuple_extension', false, true) }, + box_space_max = { + default = 'new', + obsolete = nil, + brief = BOX_SPACE_MAX_BRIEF, + action = tweak_action('BOX_SPACE_MAX', 2147483647, 2147483646) + }, } -- Array with option names in order of addition. diff --git a/test/box-luatest/fully-temporary_spaces_test.lua b/test/box-luatest/fully-temporary_spaces_test.lua index 9e6e108b48..c5e70aff86 100644 --- a/test/box-luatest/fully-temporary_spaces_test.lua +++ b/test/box-luatest/fully-temporary_spaces_test.lua @@ -62,9 +62,8 @@ g.test_temporary_create = function() s:drop() -- Now there's no more room to grow... - local BOX_SPACE_MAX = 0x7fffffff box.schema.space.create('temp2', { type = 'temporary', - id = BOX_SPACE_MAX }) + id = box.schema.SPACE_MAX }) -- ... therefore we start filling in the gaps s = box.schema.space.create('temp4', { type = 'temporary' }) diff --git a/test/box-luatest/gh_8036_create_space_with_explicit_and_implicit_id_test.lua b/test/box-luatest/gh_8036_create_space_with_explicit_and_implicit_id_test.lua index 12afd90942..e98a2ef1ee 100644 --- a/test/box-luatest/gh_8036_create_space_with_explicit_and_implicit_id_test.lua +++ b/test/box-luatest/gh_8036_create_space_with_explicit_and_implicit_id_test.lua @@ -55,13 +55,13 @@ g.test_create_space_with_explicit_and_implicit_id_space_id_overflow = function() g.server:exec(function() box.schema.space.create('SPACE', { format = {{name = 'id', type = 'unsigned'}}, - id = 0x7fffffff + id = box.schema.SPACE_MAX, }) box.schema.space.create('SPACE2', { format = {{name = 'id', type = 'unsigned'}} }) t.assert_not_equals(box.space.SPACE, nil) - t.assert_equals(box.space.SPACE.id, 0x7fffffff) + t.assert_equals(box.space.SPACE.id, box.schema.SPACE_MAX) t.assert_not_equals(box.space.SPACE2, nil) t.assert_equals(box.space.SPACE2.id, 512) end) diff --git a/test/box-luatest/gh_9118_space_max_test.lua b/test/box-luatest/gh_9118_space_max_test.lua new file mode 100644 index 0000000000..47d9744b4e --- /dev/null +++ b/test/box-luatest/gh_9118_space_max_test.lua @@ -0,0 +1,49 @@ +local server = require('luatest.server') +local t = require('luatest') + +local g = t.group() + +g.before_all(function(cg) + cg.server = server:new() + cg.server:start() +end) + +g.after_all(function(cg) + cg.server:drop() +end) + +g.after_each(function(cg) + cg.server:exec(function() + local compat = require('compat') + compat.box_space_max = 'default' + if box.space.test then + box.space.test:drop() + end + end) +end) + +g.test_space_max = function(cg) + cg.server:exec(function() + local compat = require('compat') + t.assert_equals(compat.box_space_max.current, 'default') + t.assert_equals(compat.box_space_max.default, 'new') + t.assert_equals(box.schema.SPACE_MAX, 2147483646) + t.assert_error_msg_equals( + "Failed to create space 'test': space id is too big", + box.schema.create_space, 'test', {id = 2147483647}) + local s = box.schema.create_space('test', {id = 2147483646}) + s:drop() + compat.box_space_max = 'old' + t.assert_error_msg_equals( + "Failed to create space 'test': space id is too big", + box.schema.create_space, 'test', {id = 2147483648}) + local s = box.schema.create_space('test', {id = 2147483647}) + s:drop() + compat.box_space_max = 'new' + t.assert_error_msg_equals( + "Failed to create space 'test': space id is too big", + box.schema.create_space, 'test', {id = 2147483647}) + local s = box.schema.create_space('test', {id = 2147483646}) + s:drop() + end) +end diff --git a/test/box/alter_limits.result b/test/box/alter_limits.result index 751cefe8bf..7a7109e63a 100644 --- a/test/box/alter_limits.result +++ b/test/box/alter_limits.result @@ -41,7 +41,7 @@ box.schema.INDEX_MAX ... box.schema.SPACE_MAX --- -- 2147483647 +- 2147483646 ... box.schema.SYSTEM_ID_MAX --- -- GitLab