From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: Date: Thu, 25 Apr 2019 14:43:43 +0300 From: Konstantin Osipov Subject: Re: [tarantool-patches] [PATCH v3 6/7] Add merger for tuples streams (C part) Message-ID: <20190425114343.GM29257@atlas> References: <963ad528ad35199943931150956c1d5e2c374c40.1554906327.git.alexander.turenko@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <963ad528ad35199943931150956c1d5e2c374c40.1554906327.git.alexander.turenko@tarantool.org> To: tarantool-patches@freelists.org Cc: Vladimir Davydov , Alexander Turenko List-ID: * Alexander Turenko [19/04/10 18:23]: > +enum { MERGER_SOURCE_REF_MAX = INT_MAX }; It's merge_source, not merger, source, no? Here and in the rest of the code. > +void > +merger_context_delete(struct merger_context *ctx) Please decide it's either simply merger or merge_context. merger_context does not make any sense to me. what's in the context anyway? Can you have multiple contexts in a single merge? If no, why not simply merger? Why do you need both, a merger and a context? > +/** > + * Holds immutable parameters of a merger. > + */ > +struct merger_context { > + struct key_def *key_def; > + box_tuple_format_t *format; > + /* Reference counter. */ > + int refs; > +}; Uhm.. Is it a comparison/sort context, not merger context then? Please explain why you need to ref it btw. I would simply pass it into Lua via ffi, to avoid any references, and make it clear that the life cycle of this object is bound to the merge window. -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov