From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Mon, 6 May 2019 18:46:03 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [PATCH v5 4/4] box: introduce multikey indexes in memtx
Message-ID: <20190506154603.txsxk2hsyshdqtdb@esperanza>
References: <cover.1557142159.git.kshcherbatov@tarantool.org>
 <bb1e8c779aae2538c797d45668f190ba46ce438e.1557142159.git.kshcherbatov@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <bb1e8c779aae2538c797d45668f190ba46ce438e.1557142159.git.kshcherbatov@tarantool.org>
To: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

The patch looks generally okay, but I think there's a problem re
field_map_size we have overlooked. The problem is memtx_tuple_delete
uses field_map_size to find out the allocation size:

 | void
 | 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;
 | 	tuple_format_unref(format);
 | 	struct memtx_tuple *memtx_tuple =
 | 		container_of(tuple, struct memtx_tuple, base);
 | 	if (memtx->alloc.free_mode != SMALL_DELAYED_FREE ||
 | 	    memtx_tuple->version == memtx->snapshot_version ||
 | 	    format->is_temporary)
 | 		smfree(&memtx->alloc, memtx_tuple, total);
 | 	else
 | 		smfree_delayed(&memtx->alloc, memtx_tuple, total);
 | }

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.