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 61F2824844 for ; Wed, 24 Jul 2019 18:31:44 -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 t586jSqq6Ud9 for ; Wed, 24 Jul 2019 18:31:44 -0400 (EDT) Received: from smtp46.i.mail.ru (smtp46.i.mail.ru [94.100.177.106]) (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 1C1802315F for ; Wed, 24 Jul 2019 18:31:44 -0400 (EDT) Received: by smtp46.i.mail.ru with esmtpa (envelope-from ) id 1hqPnS-000614-9a for tarantool-patches@freelists.org; Thu, 25 Jul 2019 01:31:42 +0300 Date: Thu, 25 Jul 2019 01:31:41 +0300 From: Konstantin Osipov Subject: [tarantool-patches] Re: [PATCH 2/3] vinyl: move unique check optimization setup to the space level Message-ID: <20190724223141.GB13150@atlas> References: <64b85c269c8153167e2aee302fecfab79ef58d43.1563895921.git.vdavydov.dev@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <64b85c269c8153167e2aee302fecfab79ef58d43.1563895921.git.vdavydov.dev@gmail.com> 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 * Vladimir Davydov [19/07/23 18:49]: > If any sub-set of fields indexed by a unique index is indexed by another > unique index, we can skip the uniqueness check for it. We use this to > optimize out unnecessary uniqueness checks in vinyl, where they may be > pretty costly, especially if the bloom filter doesn't reflect them. > > Currently, whether to check if an index is unique or not is determined > in vinyl space constructor, which sets vy_lsm::check_is_unique flag for > the space indexes. This looks ugly, because this means that no other > engine can make use of this optimization without duplicating the code > setting up the flags. True, no other engine needs it now, but still it > doesn't feel right. Besides, the check_is_unique flag isn't actually an > index property - it's rather a property of a space so storing it in > vy_lsm looks wrong. Because of that we have to update the flag when an > index is reassigned to another space by MoveIndex DDL operation, see > vinyl_space_swap_index(). > > So let's store the flags indicating whether a uniqueness check is > required for a particular index in a bitmap in the space struct and > set it up in the generic space constructor, space_create(). LGTM > +/** A trivial wrapper around space_build_index(). */ > +static void > +alter_space_build_index(struct space *src_space, struct space *new_space, > + struct index *new_index) > +{ > + bool check_unique = space_needs_check_unique(new_space, > + new_index->def->iid); check_unique is not proper English, please use check_unique_constraint or check_unique_ck. > + space_build_index_xc(src_space, new_index, new_space->format, > + check_unique); Why can't you move testing check_unique property inside space_build_index or even the engine specific implementation of space_build_index? > + space->unique_index_bitmap = calloc(bitmap_size(index_id_max + 1), 1); Let's use check_unique_constraint_map > +/** > + * Return true if the unique constraint must be checked for > + * the index with the given id before inserting a tuple into > + * the space. > + */ > +static inline bool > +space_needs_check_unique(struct space *space, uint32_t index_id) space_needs_unique_constraint_check -- Konstantin Osipov, Moscow, Russia