From: Vladimir Davydov <vdavydov.dev@gmail.com> To: Kirill Shcherbatov <kshcherbatov@tarantool.org> Cc: tarantool-patches@freelists.org Subject: Re: [tarantool-patches] Re: [PATCH v5 4/4] box: introduce multikey indexes in memtx Date: Tue, 7 May 2019 11:11:03 +0300 [thread overview] Message-ID: <20190507081103.fnzqguaeev4hxrlt@esperanza> (raw) In-Reply-To: <88b86a7c-686d-30ac-fe75-f892381e96a4@tarantool.org> On Mon, May 06, 2019 at 07:35:10PM +0300, Kirill Shcherbatov wrote: > > > > How's it going to work in case the field map stored in a tuple is > > greater than field_map_size? > > > > I think we should calculate the real size of the field map here > > in case the format allows multikey indexes. > > Consider this diff (not on branch yet): > > diff --git a/src/box/field_map.c b/src/box/field_map.c > index 5d25e3032..a917d4a25 100644 > --- a/src/box/field_map.c > +++ b/src/box/field_map.c > @@ -105,12 +105,28 @@ field_map_build(struct field_map_builder *builder, char *buffer) > struct field_map_builder_slot_extent *extent = > builder->slots[i].extent; > /** Retrive memory for the extent. */ > - uint32_t sz = sizeof(struct field_map_builder_slot_extent) + > - extent->size * sizeof(uint32_t); > field_map[i] = > (uint32_t)((char *)extent_wptr - (char *)field_map); > - memcpy(extent_wptr, extent, sz); > - extent_wptr += sz; > + *(uint32_t *)extent_wptr = extent->size; > + uint32_t extent_offset_sz = extent->size * sizeof(uint32_t); > + memcpy(&((uint32_t *) extent_wptr)[1], extent->offset, > + extent_offset_sz); > + extent_wptr += sizeof(uint32_t) + extent_offset_sz; > } > assert(extent_wptr == buffer + builder->extents_size); > } > + > +uint32_t > +field_map_get_size(const uint32_t *field_map, uint32_t format_field_map_sz) > +{ > + uint32_t total = format_field_map_sz; > + int32_t field_map_items = format_field_map_sz / sizeof(uint32_t); > + for (int32_t slot = -1; slot >= -field_map_items; slot--) { > + if ((int32_t)field_map[slot] >= 0) > + continue; > + uint32_t *extent = (uint32_t *)((char *)field_map + > + (int32_t)field_map[slot]); > + total += (1 + extent[0]) * sizeof(uint32_t); > + } > + return total; > +} > diff --git a/src/box/field_map.h b/src/box/field_map.h > index 0fd35c3e1..78c96e4f7 100644 > --- a/src/box/field_map.h > +++ b/src/box/field_map.h > @@ -163,6 +163,19 @@ field_map_get_offset(const uint32_t *field_map, int32_t offset_slot, > return offset; > } > > +/** > + * Get size of the tuple field_map using > + * field_map pointer and format root field_map size. > + * > + * In case of multikey indexes the real field_map size may be > + * greater than the size of format root field_map. To calculate > + * the total size of the field_map extentions, routine relies > + * on the fact that all field_map slots that has an extention > + * use negative offset as a marker. > + */ > +uint32_t > +field_map_get_size(const uint32_t *field_map, uint32_t format_field_map_sz); > + > /** > * Initialize field_map_builder. > * > diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c > index 65e8804a4..ab4913035 100644 > --- a/src/box/memtx_engine.c > +++ b/src/box/memtx_engine.c > @@ -1180,6 +1180,8 @@ memtx_tuple_new(struct tuple_format *format, const char *data, const char *end) > char *raw = (char *) tuple + tuple->data_offset; > field_map_build(&builder, raw - field_map_size); > memcpy(raw, data, tuple_len); > + assert(field_map_get_size((uint32_t *)raw, > + format->field_map_size) == field_map_size); Could you please move this assertion to field_map_build so that it works for all engines, not only for memtx? Also, we need to make sure that the key_def module doesn't allow to create multikey key definitions, as its API doesn't allow to compare multikey indexes. Please do it in the scope of this patch as well. May be, we will extend the API later to allow that, but I think we can live without it for now. > say_debug("%s(%zu) = %p", __func__, tuple_len, memtx_tuple); > end: > region_truncate(region, region_svp); > @@ -1192,8 +1194,9 @@ memtx_tuple_delete(struct tuple_format *format, struct tuple *tuple) > struct memtx_engine *memtx = (struct memtx_engine *)format->engine; > say_debug("%s(%p)", __func__, tuple); > assert(tuple->refs == 0); > - size_t total = sizeof(struct memtx_tuple) + format->field_map_size + > - tuple->bsize; > + const uint32_t *field_map = tuple_field_map(tuple); > + size_t total = sizeof(struct memtx_tuple) + tuple->bsize + > + field_map_get_size(field_map, format->field_map_size); > tuple_format_unref(format); > struct memtx_tuple *memtx_tuple = > container_of(tuple, struct memtx_tuple, base); >
next prev parent reply other threads:[~2019-05-07 8:11 UTC|newest] Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-06 11:57 [PATCH v5 0/4] " Kirill Shcherbatov 2019-05-06 11:57 ` [PATCH v5 1/4] box: introduce tuple_format_iterator class Kirill Shcherbatov 2019-05-06 13:55 ` Vladimir Davydov 2019-05-06 14:32 ` [tarantool-patches] " Kirill Shcherbatov 2019-05-06 11:57 ` [PATCH v5 2/4] box: introduce field_map_builder class Kirill Shcherbatov 2019-05-06 14:22 ` Vladimir Davydov 2019-05-06 11:57 ` [PATCH v5 3/4] salad: introduce bps_tree_delete_identical routine Kirill Shcherbatov 2019-05-06 14:34 ` Vladimir Davydov 2019-05-06 14:55 ` [tarantool-patches] " Kirill Shcherbatov 2019-05-06 11:57 ` [PATCH v5 4/4] box: introduce multikey indexes in memtx Kirill Shcherbatov 2019-05-06 15:46 ` Vladimir Davydov 2019-05-06 16:35 ` [tarantool-patches] " Kirill Shcherbatov 2019-05-07 8:11 ` Vladimir Davydov [this message] 2019-05-07 8:28 ` Kirill Shcherbatov 2019-05-07 11:30 ` Vladimir Davydov 2019-05-07 13:13 ` Vladimir Davydov 2019-05-06 15:52 ` [PATCH v5 0/4] " Vladimir Davydov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190507081103.fnzqguaeev4hxrlt@esperanza \ --to=vdavydov.dev@gmail.com \ --cc=kshcherbatov@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [tarantool-patches] Re: [PATCH v5 4/4] box: introduce multikey indexes in memtx' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox