From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id AEBCC70202; Wed, 24 Feb 2021 13:28:20 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org AEBCC70202 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1614162500; bh=k3W5PUbBPS9XaCyDtfIsw31fohfTpH8J1JMtGuPB51E=; h=To:References:Date:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=f1iXImC9f/NbSGVjplH+WYOVSXUYXWG587hz0mCfKu2AzVIZG735cLlG628I7+v+k eoTk2iymsFE2gX+wMD1Y/ohqme3GRapiF9OvqglxlNmngWZsV4z5Yekq8YLwhUDvSo yIvx3UElZoxSzRHB4KzjE/pRdemQMJllM6ujQUcg= Received: from smtp35.i.mail.ru (smtp35.i.mail.ru [94.100.177.95]) (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 1A24970202 for ; Wed, 24 Feb 2021 13:27:30 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 1A24970202 Received: by smtp35.i.mail.ru with esmtpa (envelope-from ) id 1lErOD-0007Qg-BO; Wed, 24 Feb 2021 13:27:29 +0300 To: Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org, yaroslav.dynnikov@tarantool.org References: <7a52b9f05ce9c6f2b44164668f09706c7723b104.1614039039.git.v.shpilevoy@tarantool.org> Message-ID: <542ed6d8-9b68-7455-5a79-b192078c139e@tarantool.org> Date: Wed, 24 Feb 2021 13:27:28 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.16; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <7a52b9f05ce9c6f2b44164668f09706c7723b104.1614039039.git.v.shpilevoy@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD975C3EC174F5669229511437AA01F46811CFCF616A939B362182A05F5380850402BD2C34664095737FF7DEC3F77A24500BDFB8A30FF58E5A612E4D5AE06ECFC22 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE79E25BE5045FD62C0EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637543EE1E955FF04C98638F802B75D45FF5571747095F342E8C7A0BC55FA0FE5FC37516A87862161415A6A42B1D1C7901659AFF629D847FFAF389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C0A29E2F051442AF778941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B6F459A8243F1D1D44CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB8D32BA5DBAC0009BE9E8FC8737B5C22495E9FA80E707FC84476E601842F6C81A12EF20D2F80756B5F7E9C4E3C761E06A776E601842F6C81A127C277FBC8AE2E8B0E6D796F508274C4D81D268191BDAD3D698AB9A7B718F8C442539A7722CA490C13377AFFFEAFD26923F8577A6DFFEA7CB59C7783CC88FA9693EC92FD9297F6715571747095F342E857739F23D657EF2BD5E8D9A59859A8B655EC1579764E9EAF089D37D7C0E48F6C5571747095F342E857739F23D657EF2B6825BDBE14D8E7028C9DFF55498CEFB0BD9CCCA9EDD067B1EDA766A37F9254B7 X-C1DE0DAB: 0D63561A33F958A599A1C1652A17DE697232E8A7AE37A7DA0B7B53E066BE4600D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75B7BFB303F1C7DB4D8E8E86DC7131B365E7726E8460B7C23C X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34728AF701C68E453966353087FB731C44506B06F62BD06ECC691775A4E9EB023FE1F17002BD23BCCD1D7E09C32AA3244C7914984EB9924DDD5CDE23187864177A64EE5813BBCA3A9DFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojyK6JYJ15DtIhI3Sgo3hQ0g== X-Mailru-Sender: 583F1D7ACE8F49BD9317CE1922F30C7EDD470E5413D00C48E7412AB500EF4DD1DC0049A4441B29DE23E75C7104EB1B885DEE61814008E47C7013064206BFB89F93956FB04BA385BE9437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH vshard 03/11] storage: cache bucket count X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Oleg Babin via Tarantool-patches Reply-To: Oleg Babin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Thanks for your patch! LGTM. I see calls like "status_index:count({consts.BUCKET.ACTIVE})". Maybe it worth to cache whole buckets stats as well? On 23.02.2021 03:15, Vladislav Shpilevoy wrote: > Bucket count calculation costs 1 FFI call in Lua, and makes a few > actions and virtual calls in C. So it is not free even for memtx > spaces. > > But it changes extremely rare, which makes reasonable to cache the > value. > > Bucket count is not used much now, but will be used a lot in the > future storage_ref() function, which is a part of map-reduce API. > > The idea is that a router will need to reference all the storages > and ensure that all the buckets in the cluster are pinned to their > storages. To check this, storage_ref() will return number of > buckets successfully pinned on the storage. > > The router will sum counts from all storage_ref() calls and ensure > it equals to total configured bucket count. > > This means bucket count is needed for each storage_ref() call, > whose count per second can be thousands and more. > > The patch makes count calculation cost as much as one Lua function > call and a Lua table index operation (almost always). > > Needed for #147 > --- > test/storage/storage.result | 45 +++++++++++++++++++++++++++++++++++ > test/storage/storage.test.lua | 18 ++++++++++++++ > vshard/storage/init.lua | 44 +++++++++++++++++++++++++++++----- > 3 files changed, 101 insertions(+), 6 deletions(-) > > diff --git a/test/storage/storage.result b/test/storage/storage.result > index 0550ad1..edb45be 100644 > --- a/test/storage/storage.result > +++ b/test/storage/storage.result > @@ -677,6 +677,51 @@ rs:callro('echo', {'some_data'}) > - null > - null > ... > +-- > +-- Bucket count is calculated properly. > +-- > +-- Cleanup after the previous tests. > +_ = test_run:switch('storage_1_a') > +--- > +... > +buckets = vshard.storage.buckets_info() > +--- > +... > +for bid, _ in pairs(buckets) do vshard.storage.bucket_force_drop(bid) end > +--- > +... > +_ = test_run:switch('storage_2_a') > +--- > +... > +buckets = vshard.storage.buckets_info() > +--- > +... > +for bid, _ in pairs(buckets) do vshard.storage.bucket_force_drop(bid) end > +--- > +... > +_ = test_run:switch('storage_1_a') > +--- > +... > +assert(vshard.storage.buckets_count() == 0) > +--- > +- true > +... > +vshard.storage.bucket_force_create(1, 5) > +--- > +- true > +... > +assert(vshard.storage.buckets_count() == 5) > +--- > +- true > +... > +vshard.storage.bucket_force_create(6, 5) > +--- > +- true > +... > +assert(vshard.storage.buckets_count() == 10) > +--- > +- true > +... > _ = test_run:switch("default") --- ... diff --git a/test/storage/storage.test.lua > b/test/storage/storage.test.lua index d8fbd94..db014ef 100644 --- > a/test/storage/storage.test.lua +++ b/test/storage/storage.test.lua @@ > -187,6 +187,24 @@ util.has_same_fields(old_internal, > vshard.storage.internal) _, rs = > next(vshard.storage.internal.replicasets) rs:callro('echo', > {'some_data'}) +-- +-- Bucket count is calculated properly. +-- +-- > Cleanup after the previous tests. +_ = test_run:switch('storage_1_a') > +buckets = vshard.storage.buckets_info() +for bid, _ in pairs(buckets) > do vshard.storage.bucket_force_drop(bid) end +_ = > test_run:switch('storage_2_a') +buckets = > vshard.storage.buckets_info() +for bid, _ in pairs(buckets) do > vshard.storage.bucket_force_drop(bid) end + +_ = > test_run:switch('storage_1_a') +assert(vshard.storage.buckets_count() > == 0) +vshard.storage.bucket_force_create(1, 5) > +assert(vshard.storage.buckets_count() == 5) > +vshard.storage.bucket_force_create(6, 5) > +assert(vshard.storage.buckets_count() == 10) + _ = test_run:switch("default") > test_run:drop_cluster(REPLICASET_2) > test_run:drop_cluster(REPLICASET_1) > diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua > index a3d383d..9b74bcb 100644 > --- a/vshard/storage/init.lua > +++ b/vshard/storage/init.lua > @@ -110,6 +110,9 @@ if not M then > -- replace the old function is to keep its reference. > -- > bucket_on_replace = nil, > + -- Fast alternative to box.space._bucket:count(). But may be nil. Reset > + -- on each generation change. > + bucket_count_cache = nil, > -- Redirects for recently sent buckets. They are kept for a while to > -- help routers to find a new location for sent and deleted buckets > -- without whole cluster scan. > @@ -183,10 +186,44 @@ local function local_call(func_name, args) > return pcall(netbox_self_call, netbox_self, func_name, args) > end > > +-- > +-- Get number of buckets stored on this storage. Regardless of their state. > +-- > +-- The idea is that all the code should use one function ref to get the bucket > +-- count. But inside the function never branches. Instead, it points at one of 2 > +-- branch-less functions. Cached one simply returns a number which is supposed > +-- to be super fast. Non-cached remembers the count and changes the global > +-- function to the cached one. So on the next call it is cheap. No 'if's at all. > +-- > +local bucket_count > + > +local function bucket_count_cache() > + return M.bucket_count_cache > +end > + > +local function bucket_count_not_cache() > + local count = box.space._bucket:count() > + M.bucket_count_cache = count > + bucket_count = bucket_count_cache > + return count > +end > + > +bucket_count = bucket_count_not_cache > + > +-- > +-- Can't expose bucket_count to the public API as is. Need this proxy-call. > +-- Because the original function changes at runtime. > +-- > +local function bucket_count_public() > + return bucket_count() > +end > + > -- > -- Trigger for on replace into _bucket to update its generation. > -- > local function bucket_generation_increment() > + bucket_count = bucket_count_not_cache > + M.bucket_count_cache = nil > M.bucket_generation = M.bucket_generation + 1 > M.bucket_generation_cond:broadcast() > end > @@ -2240,7 +2277,6 @@ local function rebalancer_request_state() > if #status_index:select({consts.BUCKET.GARBAGE}, {limit = 1}) > 0 then > return > end > - local bucket_count = _bucket:count() > return { > bucket_active_count = status_index:count({consts.BUCKET.ACTIVE}), > bucket_pinned_count = status_index:count({consts.BUCKET.PINNED}), > @@ -2501,10 +2537,6 @@ end > -- Monitoring > -------------------------------------------------------------------------------- > > -local function storage_buckets_count() > - return box.space._bucket.index.pk:count() > -end > - > local function storage_buckets_info(bucket_id) > local ibuckets = setmetatable({}, { __serialize = 'mapping' }) > > @@ -2780,7 +2812,7 @@ return { > cfg = function(cfg, uuid) return storage_cfg(cfg, uuid, false) end, > info = storage_info, > buckets_info = storage_buckets_info, > - buckets_count = storage_buckets_count, > + buckets_count = bucket_count_public, > buckets_discovery = buckets_discovery, > rebalancer_request_state = rebalancer_request_state, > internal = M,