From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id EE5D042EF5C for ; Thu, 18 Jun 2020 01:53:38 +0300 (MSK) References: <67c75c01-8503-2355-e1f7-9644def2179c@tarantool.org> <6b8b0f63-0a3f-d3ae-7596-2611ea433d44@tarantool.org> <6D02837E-BBEB-4CB9-806F-D5D8E66014B1@gmail.com> From: Vladislav Shpilevoy Message-ID: Date: Thu, 18 Jun 2020 00:53:37 +0200 MIME-Version: 1.0 In-Reply-To: <6D02837E-BBEB-4CB9-806F-D5D8E66014B1@gmail.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [Tarantool-patches] [PATCH 2/2] feedback: collect db engines and index features List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?0JjQu9GM0Y8g0JrQvtC90Y7RhdC+0LI=?= Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the fixes! > I’ve worked on your comments and fixed lapses. See comments below. > > Besides that, are you ok with pushing this (Lua) patch forward? I am ok. >>> +local function fill_in_features_impl(features) >>> + fill_in_space_stats(features) >>> +end >>> + >>> +local cached_schema_version = 0 >>> +local cached_feedback_features = {} >> >> 4. I would better move the cache handling into fill_in_space_stats(). >> Because when you will add more features, not related to the schema, >> they won't relate to schema version. >> >> fill_in_features() should not use any caches. Does not depend on schema. >> fill_in_space_stats() can use the cache. Because fetches info from the >> schema. >> >> I mean, it works now, but we would need to rewrite that mostly, when >> more features will be collected. > > Agree. Extracted out caching into fill_in_space_stats(), but kept it in fill_in_features(), since space_stats is semantically features. This is a little bit better but basically is still the same. Just add a few dummy functions like fill_in_swim() fill_in_box_update() fill_in_vinyl_stat() fill_in_netbox() which would need to add several new fields to the feedback.features table, and see how hard it is to fit it into this code without rewriting it completely. Ideally addition of such new functions should not change a single line of the existing code. Perhaps you may need to store schema features in feedback.features.schema, not in feedback.features. So the schema becomes isolated from the other features. And store the others like feedback.features.swim, feedback.features.box_update, feedback.features.vinyl, etc. Not in a huge flat table.