diff --git a/include/scoped_guard.h b/include/scoped_guard.h index 1aed2882feb6a2e61b42c0c173eea6599d8fa72a..63698bdafc3fcf3d42106f4899e9b60c666d73c6 100644 --- a/include/scoped_guard.h +++ b/include/scoped_guard.h @@ -33,33 +33,30 @@ #include "object.h" template <typename Functor> -class ScopedGuard { -public: +struct ScopedGuard { + Functor f; + bool is_active; + explicit ScopedGuard(const Functor& fun) - : m_fun(fun), m_active(true) { + : f(fun), is_active(true) { /* nothing */ } ScopedGuard(ScopedGuard&& guard) - : m_fun(guard.m_fun), m_active(true) { - guard.m_active = false; + : f(guard.f), is_active(true) { + guard.is_active = false; abort(); } ~ScopedGuard() { - if (!m_active) - return; - - m_fun(); + if (is_active) + f(); } private: explicit ScopedGuard(const ScopedGuard&) = delete; ScopedGuard& operator=(const ScopedGuard&) = delete; - - Functor m_fun; - bool m_active; }; template <typename Functor> diff --git a/src/box/alter.cc b/src/box/alter.cc index d8a988cc120a0bfaa74f5ebbcfcfebc23ccb5541..f60fd1030487f5126a3187cda4648b58778a74ce 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -32,6 +32,7 @@ #include "txn.h" #include "tuple.h" #include "fiber.h" /* for gc_pool */ +#include "scoped_guard.h" #include <new> /* for placement new */ #include <stdio.h> /* snprintf() */ @@ -71,25 +72,24 @@ key_def_new_from_tuple(struct tuple *tuple) struct key_def *key_def = key_def_new(index_id, type, is_unique > 0, part_count); - try { - struct tuple_iterator it; - tuple_rewind(&it, tuple); - uint32_t len; /* unused */ - /* Parts follow part count. */ - (void) tuple_seek(&it, INDEX_PART_COUNT, &len); + auto scoped_guard = + make_scoped_guard([=] { key_def_delete(key_def); }); - for (uint32_t i = 0; i < part_count; i++) { - uint32_t fieldno = tuple_next_u32(&it); - const char *field_type_str = tuple_next_cstr(&it); - enum field_type field_type = - STR2ENUM(field_type, field_type_str); - key_def_set_part(key_def, i, fieldno, field_type); - } - key_def_check(id, key_def); - } catch (...) { - key_def_delete(key_def); - throw; + struct tuple_iterator it; + tuple_rewind(&it, tuple); + uint32_t len; /* unused */ + /* Parts follow part count. */ + (void) tuple_seek(&it, INDEX_PART_COUNT, &len); + + for (uint32_t i = 0; i < part_count; i++) { + uint32_t fieldno = tuple_next_u32(&it); + const char *field_type_str = tuple_next_cstr(&it); + enum field_type field_type = + STR2ENUM(field_type, field_type_str); + key_def_set_part(key_def, i, fieldno, field_type); } + key_def_check(id, key_def); + scoped_guard.is_active = false; return key_def; } @@ -939,17 +939,15 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) * in WAL-error-safe mode. */ struct alter_space *alter = alter_space_new(); - try { - ModifySpace *modify = - AlterSpaceOp::create<ModifySpace>(); - alter_space_add_op(alter, modify); - space_def_create_from_tuple(&modify->def, new_tuple, - ER_ALTER_SPACE); - alter_space_do(txn, alter, old_space); - } catch (...) { - alter_space_delete(alter); - throw; - } + auto scoped_guard = + make_scoped_guard([=] {alter_space_delete(alter);}); + ModifySpace *modify = + AlterSpaceOp::create<ModifySpace>(); + alter_space_add_op(alter, modify); + space_def_create_from_tuple(&modify->def, new_tuple, + ER_ALTER_SPACE); + alter_space_do(txn, alter, old_space); + scoped_guard.is_active = false; } } @@ -1001,27 +999,25 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) struct space *old_space = space_find(id); Index *old_index = space_index(old_space, index_id); struct alter_space *alter = alter_space_new(); - try { - /* - * The order of checks is important, DropIndex most be added - * first, so that AddIndex::prepare() can change - * Drop + Add to a Modify. - */ - if (old_index != NULL) { - DropIndex *drop_index = AlterSpaceOp::create<DropIndex>(); - alter_space_add_op(alter, drop_index); - drop_index->old_key_def = old_index->key_def; - } - if (new_tuple != NULL) { - AddIndex *add_index = AlterSpaceOp::create<AddIndex>(); - alter_space_add_op(alter, add_index); - add_index->new_key_def = key_def_new_from_tuple(new_tuple); - } - alter_space_do(txn, alter, old_space); - } catch (...) { - alter_space_delete(alter); - throw; + auto scoped_guard = + make_scoped_guard([=] { alter_space_delete(alter); }); + /* + * The order of checks is important, DropIndex most be added + * first, so that AddIndex::prepare() can change + * Drop + Add to a Modify. + */ + if (old_index != NULL) { + DropIndex *drop_index = AlterSpaceOp::create<DropIndex>(); + alter_space_add_op(alter, drop_index); + drop_index->old_key_def = old_index->key_def; + } + if (new_tuple != NULL) { + AddIndex *add_index = AlterSpaceOp::create<AddIndex>(); + alter_space_add_op(alter, add_index); + add_index->new_key_def = key_def_new_from_tuple(new_tuple); } + alter_space_do(txn, alter, old_space); + scoped_guard.is_active = false; } struct trigger alter_space_on_replace_space = { diff --git a/src/box/box_lua.cc b/src/box/box_lua.cc index 008ad8aa642ff1b8103dcc1a46d9db24fa544f84..839122c32cf8b435f57eacab3ee2913339b8ba94 100644 --- a/src/box/box_lua.cc +++ b/src/box/box_lua.cc @@ -1877,6 +1877,8 @@ schema_lua_init(struct lua_State *L) lua_setfield(L, -2, "FIELD_MAX"); lua_pushnumber(L, BOX_NAME_MAX); lua_setfield(L, -2, "NAME_MAX"); + lua_pushnumber(L, FORMAT_ID_MAX); + lua_setfield(L, -2, "FORMAT_ID_MAX"); lua_pop(L, 2); /* box, schema */ } diff --git a/src/box/index.cc b/src/box/index.cc index d6b81721a8d5b65e8326570716d492f5d16c2898..05c79c90892ee2d0adff88ccada47078826a484a 100644 --- a/src/box/index.cc +++ b/src/box/index.cc @@ -113,19 +113,18 @@ Index::factory(struct key_def *key_def) { switch (key_def->type) { case HASH: - return new (std::nothrow) HashIndex(key_def); + return new HashIndex(key_def); case TREE: - return new (std::nothrow) TreeIndex(key_def); + return new TreeIndex(key_def); case BITSET: - return new (std::nothrow) BitsetIndex(key_def); + return new BitsetIndex(key_def); default: assert(false); } - return NULL; } Index::Index(struct key_def *key_def_arg) - :key_def(key_def_arg), + :key_def(key_def_dup(key_def_arg)), m_position(NULL) {} diff --git a/src/box/key_def.cc b/src/box/key_def.cc index fa06265a62579ac14c1b64b270bb7e01e2360bcc..4bad78aa2a87e97d740842945d05202c1a79eda7 100644 --- a/src/box/key_def.cc +++ b/src/box/key_def.cc @@ -38,8 +38,11 @@ key_def_new(uint32_t id, enum index_type type, bool is_unique, uint32_t part_count) { uint32_t parts_size = sizeof(struct key_part) * part_count; - struct key_def *def = (struct key_def *) - malloc(parts_size + sizeof(*def)); + size_t sz = parts_size + sizeof(struct key_def); + struct key_def *def = (struct key_def *) malloc(sz); + if (def == NULL) + tnt_raise(LoggedError, ER_MEMORY_ISSUE, + sz, "struct key_def", "malloc"); def->type = type; def->id = id; def->is_unique = is_unique; diff --git a/src/box/space.cc b/src/box/space.cc index 5cc0e5d6884fcee676f819d7ba70323b34ce8aeb..2b08ad60366a872a9a4f5ab6495f017e3562b14f 100644 --- a/src/box/space.cc +++ b/src/box/space.cc @@ -31,6 +31,7 @@ #include <string.h> #include <exception.h> #include "tuple.h" +#include "scoped_guard.h" void space_fill_index_map(struct space *space) @@ -44,7 +45,7 @@ space_fill_index_map(struct space *space) } struct space * -space_new(struct space_def *space_def, struct rlist *key_list) +space_new(struct space_def *def, struct rlist *key_list) { uint32_t index_id_max = 0; uint32_t index_count = 0; @@ -58,34 +59,31 @@ space_new(struct space_def *space_def, struct rlist *key_list) struct space *space = (struct space *) calloc(1, sz); if (space == NULL) - return NULL; + tnt_raise(LoggedError, ER_MEMORY_ISSUE, + sz, "struct space", "malloc"); + + auto scoped_guard = make_scoped_guard([=] + { + space_fill_index_map(space); + space_delete(space); + }); space->index_map = (Index **)((char *) space + sizeof(*space) + index_count * sizeof(Index *)); - space->def = *space_def; + space->def = *def; space->format = tuple_format_new(key_list); tuple_format_ref(space->format, 1); space->index_id_max = index_id_max; /* fill space indexes */ rlist_foreach_entry(key_def, key_list, link) { - struct key_def *dup = key_def_dup(key_def); - if (dup == NULL) - goto error; - Index *index = Index::factory(dup); - if (index == NULL) { - key_def_delete(dup); - goto error; - } - space->index_map[key_def->id] = index; + space->index_map[key_def->id] = Index::factory(key_def); } space_fill_index_map(space); space->engine = engine_no_keys; rlist_create(&space->on_replace); space->run_triggers = true; + scoped_guard.is_active = false; return space; -error: - space_delete(space); - return NULL; } void @@ -93,7 +91,8 @@ space_delete(struct space *space) { for (uint32_t j = 0; j < space->index_count; j++) delete space->index[j]; - tuple_format_ref(space->format, -1); + if (space->format) + tuple_format_ref(space->format, -1); free(space); } diff --git a/src/box/tuple.cc b/src/box/tuple.cc index 5eebf1ab249085d3d1bcc334f1b4f15c8803cd3a..33375d7cc89555b5a4727ae47068b6a0182c4bbf 100644 --- a/src/box/tuple.cc +++ b/src/box/tuple.cc @@ -37,7 +37,6 @@ #include <stdio.h> #include <third_party/base64.h> -enum { FORMAT_ID_MAX = UINT16_MAX - 1, FORMAT_ID_NIL = UINT16_MAX }; /** Global table of tuple formats */ struct tuple_format **tuple_formats; @@ -80,27 +79,26 @@ tuple_format_register(struct tuple_format *format) format->id = (uint16_t) recycled_format_ids; recycled_format_ids = (intptr_t) tuple_formats[recycled_format_ids]; - } else if (formats_size == formats_capacity) { - uint32_t new_capacity = formats_capacity ? - formats_capacity * 2 : 16; - struct tuple_format **formats; - if (new_capacity >= FORMAT_ID_MAX) { + } else { + if (formats_size == formats_capacity) { + uint32_t new_capacity = formats_capacity ? + formats_capacity * 2 : 16; + struct tuple_format **formats; + formats = (struct tuple_format **) + realloc(tuple_formats, new_capacity * + sizeof(tuple_formats[0])); + if (formats == NULL) + tnt_raise(LoggedError, ER_MEMORY_ISSUE, + sizeof(struct tuple_format), + "tuple_formats", "malloc"); + + formats_capacity = new_capacity; + tuple_formats = formats; + } + if (formats_size == FORMAT_ID_MAX + 1) { tnt_raise(LoggedError, ER_TUPLE_FORMAT_LIMIT, - (unsigned) new_capacity); + (unsigned) formats_capacity); } - formats = (struct tuple_format **) - realloc(tuple_formats, new_capacity * - sizeof(tuple_formats[0])); - if (formats == NULL) - tnt_raise(LoggedError, ER_MEMORY_ISSUE, - sizeof(struct tuple_format), - "tuple_formats", "malloc"); - - formats_capacity = new_capacity; - tuple_formats = formats; - - format->id = formats_size++; - } else { format->id = formats_size++; } tuple_formats[format->id] = format; diff --git a/src/box/tuple.h b/src/box/tuple.h index 4974b47c08cbf0757bd9a93ff4ffd30adbe32fbf..0297dc7677fc3d2b4eb8aa1c57497b85a528123c 100644 --- a/src/box/tuple.h +++ b/src/box/tuple.h @@ -32,6 +32,8 @@ #include "key_def.h" /* for enum field_type */ #include <pickle.h> +enum { FORMAT_ID_MAX = UINT16_MAX - 1, FORMAT_ID_NIL = UINT16_MAX }; + struct tbuf; /** @@ -106,7 +108,7 @@ tuple_format_id(struct tuple_format *format) * format. * @param space description * - * @return tuple format + * @return tuple format or raise an exception on error */ struct tuple_format * tuple_format_new(struct rlist *key_list); diff --git a/test/box/alter_limits.result b/test/box/alter_limits.result index 819712b68c294b7b9c4e8366075ad61ad2d53c0e..37683bbc42420c31a8e4d4610ecac775d2958b6f 100644 --- a/test/box/alter_limits.result +++ b/test/box/alter_limits.result @@ -37,6 +37,10 @@ box.schema.SCHEMA_ID --- - 272 ... +box.schema.FORMAT_ID_MAX +--- +- 65534 +... -- ---------------------------------------------------------------- -- CREATE SPACE -- ---------------------------------------------------------------- @@ -305,26 +309,36 @@ s:select(0) - [3, 4, 5] - [7, 8, 9] ... -s:drop() +-- check transition of space from enabled to disabled on +-- deletion of the primary key +s.enabled --- +- true ... --- Test plan --- -------- --- add space: --- --------- --- - arity change - empty, non-empty space --- - test that during commit phase --- -> inject error at commit, inject error at rollback --- - check transition of a space to "disabled" on +s.index[0]:drop() +--- +... +s.enabled +--- +- false +... +s.index[0] +--- +- null +... +-- "disabled" on -- deletion of primary key --- wal: --- - too many spaces --- - find out why we run short of 32k formats --- - longevity test for create/drop --- +s:drop() +--- +... +-- -- inject error at various stages of commit and see that +-- the alter has no effects -- -- add index: -- --------- +-- - a test case for checks in tuple_format_new +-- - it raises an exception, and there may be a memory +-- leak if it is not handled correctly -- - a test case for every constraint in key_def_check -- - test that during the rebuild there is a duplicate -- according to the new index diff --git a/test/box/alter_limits.test.lua b/test/box/alter_limits.test.lua index 92c44ea425e4d1557912570faceda813fb7acf1b..82e3f0a0fd54edee25d9100118fe505f2b96be46 100644 --- a/test/box/alter_limits.test.lua +++ b/test/box/alter_limits.test.lua @@ -10,6 +10,7 @@ box.schema.INDEX_MAX box.schema.SPACE_MAX box.schema.SYSTEM_ID_MAX box.schema.SCHEMA_ID +box.schema.FORMAT_ID_MAX -- ---------------------------------------------------------------- -- CREATE SPACE -- ---------------------------------------------------------------- @@ -104,25 +105,25 @@ s:insert(3, 4, 5) s:insert(3, 4, 5, 6) s:insert(7, 8, 9) s:select(0) -s:drop() +-- check transition of space from enabled to disabled on +-- deletion of the primary key +s.enabled +s.index[0]:drop() +s.enabled +s.index[0] --- Test plan --- -------- --- add space: --- --------- --- - arity change - empty, non-empty space --- - test that during commit phase --- -> inject error at commit, inject error at rollback --- - check transition of a space to "disabled" on +-- "disabled" on -- deletion of primary key --- wal: --- - too many spaces --- - find out why we run short of 32k formats --- - longevity test for create/drop --- +s:drop() + +-- -- inject error at various stages of commit and see that +-- the alter has no effects -- -- add index: -- --------- +-- - a test case for checks in tuple_format_new +-- - it raises an exception, and there may be a memory +-- leak if it is not handled correctly -- - a test case for every constraint in key_def_check -- - test that during the rebuild there is a duplicate -- according to the new index diff --git a/test/wal/alter.result b/test/wal/alter.result new file mode 100644 index 0000000000000000000000000000000000000000..6422a461212e534be93d987b5e00ff48397562c6 --- /dev/null +++ b/test/wal/alter.result @@ -0,0 +1,27 @@ +-- wal is off, good opportunity to test something more CPU intensive: +spaces = {} +--- +... +box.schema.FORMAT_ID_MAX +--- +- 65534 +... +--# setopt delimiter ';' +-- too many formats +for k = 1, box.schema.FORMAT_ID_MAX, 1 do + local s = box.schema.create_space('space'..k) + table.insert(spaces, s) +end; +--- +- error: 'Tuple format limit reached: 65536' +... +#spaces; +--- +- 65526 +... +-- cleanup +for k, v in pairs(spaces) do + v:drop() +end; +--- +... diff --git a/test/wal/alter.test.lua b/test/wal/alter.test.lua new file mode 100644 index 0000000000000000000000000000000000000000..d9f5430c6ed68be6310aa355952bdff510d51831 --- /dev/null +++ b/test/wal/alter.test.lua @@ -0,0 +1,14 @@ +-- wal is off, good opportunity to test something more CPU intensive: +spaces = {} +box.schema.FORMAT_ID_MAX +--# setopt delimiter ';' +-- too many formats +for k = 1, box.schema.FORMAT_ID_MAX, 1 do + local s = box.schema.create_space('space'..k) + table.insert(spaces, s) +end; +#spaces; +-- cleanup +for k, v in pairs(spaces) do + v:drop() +end; diff --git a/test/wal/oom.result b/test/wal/oom.result index d3294afdd8517aa970a45a9619327d921d04a844..3314ecc906eea5f2e0864c5de51fa8fbd95b1c5c 100644 --- a/test/wal/oom.result +++ b/test/wal/oom.result @@ -1,3 +1,5 @@ +--# stop server default +--# start server default box.insert(box.schema.SPACE_ID, 0, 0, 'tweedledum') --- - [0, 0, 'tweedledum'] diff --git a/test/wal/oom.test.lua b/test/wal/oom.test.lua index 3aed895db599bf4837465ce052f9a6460bcd9fa3..9092ff979baddfae771a4e0f0c25a5ee12413caa 100644 --- a/test/wal/oom.test.lua +++ b/test/wal/oom.test.lua @@ -1,3 +1,5 @@ +--# stop server default +--# start server default box.insert(box.schema.SPACE_ID, 0, 0, 'tweedledum') box.insert(box.schema.INDEX_ID, 0, 0, 'primary', 'hash', 1, 1, 0, 'num') space = box.space[0]