From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Kirill Shcherbatov Subject: Re: [tarantool-patches] Re: [PATCH v5 11/12] box: introduce offset slot cache in key_part References: <20181101133222.GB30032@chai> Message-ID: <5c111ccd-d2d6-1d67-187c-b6fb5be9cfa6@tarantool.org> Date: Tue, 6 Nov 2018 15:15:22 +0300 MIME-Version: 1.0 In-Reply-To: <20181101133222.GB30032@chai> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit To: tarantool-patches@freelists.org, Kostya Osipov Cc: Vladimir Davydov List-ID: > I don't understand this comment. Could you please rephrase? > I don't understand this statement either. Could you give an > example? > Did you consider storing it elsewhere, e.g. in some kind of > index search context? Tuned tuple_field_by_part_raw routine with key_part's offset_slot_cache. Introduced tuple_format epoch to test validity of this cache. The key_part caches last offset_slot source format to make epoch comparison because same space may have multiple format of same epoch that have different key_parts and related offset_slots distribution. >> - alter->new_space = space_new_xc(alter->space_def, &alter->key_list); >> + alter->new_space = >> + space_new_xc(alter->space_def, &alter->key_list, >> + alter->old_space->format != NULL ? >> + alter->old_space->format->epoch + 1 : 1); > > Can't we make it simpler and simply increase epoch id every > time we create a new space? This is only an optimization, > by leaking it into alter.cc you are making alter worry about > stuff which should not be its concern.> Passing epoch id around explicitly is ugly. The main problem we faced is that there are two formats in vinyl - regular and disk format. Alter.cc does not know anything about the internal structure of the vinyl space object, so he cannot update the disk epoch on his own. > Why can't you do it in key_def_set_part? > Lack of code reuse, abstraction leak. Like this for now: key_def_set_part(new_def, pos++, part->fieldno, part->type, part->nullable_action, part->coll, part->coll_id, part->sort_order, part->path, part->path_len, part->offset_slot_cache, part->format_cache); > The source format to check that offset_slot_epoch is not stale. > > Please avoid using the word "actuality". "hit"