From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id D4A0B298A8 for ; Sun, 8 Apr 2018 09:56:42 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8SDx1mQb7-X5 for ; Sun, 8 Apr 2018 09:56:42 -0400 (EDT) Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [94.100.177.104]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id ECBE029875 for ; Sun, 8 Apr 2018 09:56:41 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 2/2] Fixed lost format on update and upsert operations. References: From: Vladislav Shpilevoy Message-ID: <1f4e0a5e-30f2-f36a-3970-ec4eeb14a456@tarantool.org> Date: Sun, 8 Apr 2018 16:56:38 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org, Kirill Shcherbatov As we discussed verbally - Kirill will try to inline box_tuple_update into tuple:transform() to avoid creating box_tuple_update_ex(). On 06/04/2018 19:24, Kirill Shcherbatov wrote: > Fixes #3051 > --- > src/box/lua/tuple.c | 4 ++- > src/box/memtx_tuple.cc | 1 + > src/box/tuple.c | 42 ++++++++++++++++-------- > src/box/tuple.h | 6 +++- > src/box/tuple_format.h | 5 ++- > src/box/vy_stmt.c | 7 ++++ > src/box/vy_stmt.h | 9 +++++ > test/box/update.result | 85 ++++++++++++++++++++++++++++++++++++++++++++++++ > test/box/update.test.lua | 28 ++++++++++++++++ > 9 files changed, 170 insertions(+), 17 deletions(-) > > diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c > index 47b33c9..dffd5a5 100644 > --- a/src/box/lua/tuple.c > +++ b/src/box/lua/tuple.c > @@ -36,6 +36,7 @@ > #include > #include > #include > +#include > > #include "box/tuple.h" > #include "box/tuple_convert.h" > @@ -392,7 +393,8 @@ lbox_tuple_transform(struct lua_State *L) > > /* Execute tuple_update */ > struct tuple *new_tuple = > - box_tuple_update(tuple, buf->buf, buf->buf + ibuf_used(buf)); > + box_tuple_update_ex(tuple, buf->buf, buf->buf + ibuf_used(buf), > + box_tuple_format_default()); > if (new_tuple == NULL) > luaT_error(L); > /* box_tuple_update() doesn't leak on exception, see public API doc */ > diff --git a/src/box/memtx_tuple.cc b/src/box/memtx_tuple.cc > index 70453bd..1990bdf 100644 > --- a/src/box/memtx_tuple.cc > +++ b/src/box/memtx_tuple.cc > @@ -90,6 +90,7 @@ memtx_tuple_free(void) > > struct tuple_format_vtab memtx_tuple_format_vtab = { > memtx_tuple_delete, > + memtx_tuple_new, > }; > > struct tuple * > diff --git a/src/box/tuple.c b/src/box/tuple.c > index d4760f3..ec2c3b5 100644 > --- a/src/box/tuple.c > +++ b/src/box/tuple.c > @@ -61,15 +61,19 @@ struct tuple_format *tuple_format_runtime; > static void > runtime_tuple_delete(struct tuple_format *format, struct tuple *tuple); > > +struct tuple * > +runtime_tuple_new(struct tuple_format *format, const char *data, const char *end); > + > /** A virtual method table for tuple_format_runtime */ > static struct tuple_format_vtab tuple_format_runtime_vtab = { > runtime_tuple_delete, > + runtime_tuple_new, > }; > > struct tuple * > -tuple_new(struct tuple_format *format, const char *data, const char *end) > +runtime_tuple_new(struct tuple_format *format, const char *data, const char *end) > { > - assert(format->vtab.destroy == tuple_format_runtime_vtab.destroy); > + assert(format->vtab.tuple_delete == tuple_format_runtime_vtab.tuple_delete); > > mp_tuple_assert(data, end); > size_t data_len = end - data; > @@ -102,7 +106,7 @@ tuple_new(struct tuple_format *format, const char *data, const char *end) > static void > runtime_tuple_delete(struct tuple_format *format, struct tuple *tuple) > { > - assert(format->vtab.destroy == tuple_format_runtime_vtab.destroy); > + assert(format->vtab.tuple_delete == tuple_format_runtime_vtab.tuple_delete); > say_debug("%s(%p)", __func__, tuple); > assert(tuple->refs == 0); > size_t total = sizeof(struct tuple) + tuple_format_meta_size(format) + > @@ -147,6 +151,12 @@ tuple_validate_raw(struct tuple_format *format, const char *tuple) > return 0; > } > > +struct tuple * > +tuple_new(struct tuple_format *format, const char *data, const char *end) > +{ > + return format->vtab.tuple_new(format, data, end); > +} > + > /** > * Incremented on every snapshot and is used to distinguish tuples > * which were created after start of a snapshot (these tuples can > @@ -393,25 +403,23 @@ box_tuple_next(box_tuple_iterator_t *it) > } > > box_tuple_t * > -box_tuple_update(const box_tuple_t *tuple, const char *expr, > - const char *expr_end) > +box_tuple_update_ex(const box_tuple_t *tuple, const char *expr, > + const char *expr_end, struct tuple_format *format) > { > - struct tuple_format *format = tuple_format_runtime; > - > uint32_t new_size = 0, bsize; > const char *old_data = tuple_data_range(tuple, &bsize); > struct region *region = &fiber()->gc; > size_t used = region_used(region); > const char *new_data = > tuple_update_execute(region_aligned_alloc_cb, region, expr, > - expr_end, old_data, old_data + bsize, > - &new_size, 1, NULL); > + expr_end, old_data, old_data + bsize, > + &new_size, 1, NULL); > if (new_data == NULL) { > region_truncate(region, used); > return NULL; > } > - > - struct tuple *ret = tuple_new(format, new_data, new_data + new_size); > + struct tuple *ret = tuple_new(format, new_data, > + new_data + new_size); > region_truncate(region, used); > if (ret != NULL) > return tuple_bless(ret); > @@ -419,11 +427,16 @@ box_tuple_update(const box_tuple_t *tuple, const char *expr, > } > > box_tuple_t * > -box_tuple_upsert(const box_tuple_t *tuple, const char *expr, > +box_tuple_update(const box_tuple_t *tuple, const char *expr, > const char *expr_end) > { > - struct tuple_format *format = tuple_format_runtime; > + return box_tuple_update_ex(tuple, expr, expr_end, tuple_format(tuple)); > +} > > +box_tuple_t * > +box_tuple_upsert(const box_tuple_t *tuple, const char *expr, > + const char *expr_end) > +{ > uint32_t new_size = 0, bsize; > const char *old_data = tuple_data_range(tuple, &bsize); > struct region *region = &fiber()->gc; > @@ -437,7 +450,8 @@ box_tuple_upsert(const box_tuple_t *tuple, const char *expr, > return NULL; > } > > - struct tuple *ret = tuple_new(format, new_data, new_data + new_size); > + struct tuple *ret = tuple_new(tuple_format(tuple), > + new_data, new_data + new_size); > region_truncate(region, used); > if (ret != NULL) > return tuple_bless(ret); > diff --git a/src/box/tuple.h b/src/box/tuple.h > index 6ebedf5..d5b5bdb 100644 > --- a/src/box/tuple.h > +++ b/src/box/tuple.h > @@ -282,6 +282,10 @@ box_tuple_update(const box_tuple_t *tuple, const char *expr, const > char *expr_end); > > box_tuple_t * > +box_tuple_update_ex(const box_tuple_t *tuple, const char *expr, > + const char *expr_end, struct tuple_format *format); > + > +box_tuple_t * > box_tuple_upsert(const box_tuple_t *tuple, const char *expr, const > char *expr_end); > > @@ -445,7 +449,7 @@ tuple_delete(struct tuple *tuple) > say_debug("%s(%p)", __func__, tuple); > assert(tuple->refs == 0); > struct tuple_format *format = tuple_format(tuple); > - format->vtab.destroy(format, tuple); > + format->vtab.tuple_delete(format, tuple); > } > > /** > diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h > index d35182d..cc727ec 100644 > --- a/src/box/tuple_format.h > +++ b/src/box/tuple_format.h > @@ -68,7 +68,10 @@ struct tuple_format; > struct tuple_format_vtab { > /** Free allocated tuple using engine-specific memory allocator. */ > void > - (*destroy)(struct tuple_format *format, struct tuple *tuple); > + (*tuple_delete)(struct tuple_format *format, struct tuple *tuple); > + /** Allocates a new tuple on the same allocator and with the same format. */ > + struct tuple* > + (*tuple_new)(struct tuple_format *format, const char *data, const char *end); > }; > > /** Tuple field meta information for tuple_format. */ > diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c > index 84182e7..3135c58 100644 > --- a/src/box/vy_stmt.c > +++ b/src/box/vy_stmt.c > @@ -47,6 +47,7 @@ > > struct tuple_format_vtab vy_tuple_format_vtab = { > vy_tuple_delete, > + vy_tuple_new, > }; > > size_t vy_max_tuple_size = 1024 * 1024; > @@ -69,6 +70,12 @@ vy_tuple_delete(struct tuple_format *format, struct tuple *tuple) > free(tuple); > } > > +struct tuple * > +vy_tuple_new(struct tuple_format *format, const char *data, const char *end) > +{ > + return vy_stmt_new_insert(format, data, end); > +} > + > /** > * Allocate a vinyl statement object on base of the struct tuple > * with malloc() and the reference counter equal to 1. > diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h > index a33739d..efc99c8 100644 > --- a/src/box/vy_stmt.h > +++ b/src/box/vy_stmt.h > @@ -206,6 +206,15 @@ void > vy_tuple_delete(struct tuple_format *format, struct tuple *tuple); > > /** > + * Create the tuple of a vinyl space. > + * @param format Target tuple format. > + * @param data Target tuple data. > + * @param size Size of the data. > + */ > +struct tuple * > +vy_tuple_new(struct tuple_format *format, const char *data, const char *end); > + > +/** > * Duplicate the statememnt. > * > * @param stmt statement > diff --git a/test/box/update.result b/test/box/update.result > index a3f731b..b8e94b1 100644 > --- a/test/box/update.result > +++ b/test/box/update.result > @@ -849,3 +849,88 @@ s:update(1, {{'=', 3, map}}) > s:drop() > --- > ... > +format = {} > +--- > +... > +format[1] = {name = 'KEY', type = 'unsigned'} > +--- > +... > +format[2] = {name = 'VAL', type = 'string'} > +--- > +... > +s = box.schema.create_space('tst_sample', {format = format}) > +--- > +... > +pk = s:create_index('pk') > +--- > +... > +s:insert({1, 'sss'}) > +--- > +- [1, 'sss'] > +... > +aa = box.space.tst_sample:get(1) > +--- > +... > +aa.VAL > +--- > +- sss > +... > +aa = aa:update({{'=',2,'ssss'}}) > +--- > +... > +collectgarbage() > +--- > +- 0 > +... > +aa.VAL > +--- > +- ssss > +... > +s:drop() > +--- > +... > +aa = nil > +--- > +... > +collectgarbage() > +--- > +- 0 > +... > +s = box.schema.space.create('tst_sample', {engine = 'vinyl', format = format}) > +--- > +... > +pk = s:create_index('pk') > +--- > +... > +s:insert({1, 'sss'}) > +--- > +- [1, 'sss'] > +... > +aa = box.space.tst_sample:get(1) > +--- > +... > +aa.VAL > +--- > +- sss > +... > +aa = aa:update({{'=',2,'ssss'}}) > +--- > +... > +collectgarbage() > +--- > +- 0 > +... > +aa.VAL > +--- > +- ssss > +... > +s:drop() > +--- > +... > +aa = nil > +--- > +... > +collectgarbage() > +--- > +- 0 > +... > diff --git a/test/box/update.test.lua b/test/box/update.test.lua > index c455bc1..37be3a0 100644 > --- a/test/box/update.test.lua > +++ b/test/box/update.test.lua > @@ -269,3 +269,31 @@ t:update({{'=', 3, map}}) > s:update(1, {{'=', 3, map}}) > > s:drop() > + > + > +format = {} > +format[1] = {name = 'KEY', type = 'unsigned'} > +format[2] = {name = 'VAL', type = 'string'} > +s = box.schema.create_space('tst_sample', {format = format}) > +pk = s:create_index('pk') > +s:insert({1, 'sss'}) > +aa = box.space.tst_sample:get(1) > +aa.VAL > +aa = aa:update({{'=',2,'ssss'}}) > +collectgarbage() > +aa.VAL > +s:drop() > +aa = nil > +collectgarbage() > + > +s = box.schema.space.create('tst_sample', {engine = 'vinyl', format = format}) > +pk = s:create_index('pk') > +s:insert({1, 'sss'}) > +aa = box.space.tst_sample:get(1) > +aa.VAL > +aa = aa:update({{'=',2,'ssss'}}) > +collectgarbage() > +aa.VAL > +s:drop() > +aa = nil > +collectgarbage() >