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 C5EB67030F; Thu, 25 Feb 2021 00:50:15 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C5EB67030F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1614203415; bh=jdlgTSLthAZjXR821jUGyA4jCiT4Nw51KczTdO9NLYg=; 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=cTgNZ6UQVYMBYPTp3fIvd02Y0Eip1A/VIsohCtGKRZKF4XxvpVzDc4dDfAHsyCOI1 a/vFX/jYz10041uFSs9ZQxtrSbn1AnFi6Vpqm2gZe/KsGp9exIGEzoJej/9kuxRaEz 9CQpfT6qQMS9boUclXLRB2yRG8Bc/ECKJ31T2Z38= Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [217.69.128.34]) (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 95D7D7030F for ; Thu, 25 Feb 2021 00:50:04 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 95D7D7030F Received: by smtp54.i.mail.ru with esmtpa (envelope-from ) id 1lF22l-0005uh-Q7; Thu, 25 Feb 2021 00:50:04 +0300 To: Oleg Babin , tarantool-patches@dev.tarantool.org, yaroslav.dynnikov@tarantool.org References: <02df9747667f254ee5734cfafa681eead566f212.1614039039.git.v.shpilevoy@tarantool.org> <5e011bc2-426b-e59a-f165-fef74f998b18@tarantool.org> Message-ID: <214fcc16-f57c-71be-fe5a-473ad6be2c10@tarantool.org> Date: Wed, 24 Feb 2021 22:50:02 +0100 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <5e011bc2-426b-e59a-f165-fef74f998b18@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD975C3EC174F56692243410BA6471F0166336C1783AA96243D182A05F538085040288AFC105FCBCAFB6CE1AA5A4A2B0984426BE8BB4A5AC4115CB3AA57BF519C2C X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7495A032B936E882FEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063706922F90966A37BA8638F802B75D45FF914D58D5BE9E6BC131B5C99E7648C95CDDE882590F889B1CDD17FE439AE06624E079259E1C33CAD8A471835C12D1D9774AD6D5ED66289B5278DA827A17800CE77A825AB47F0FC8649FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C390D92131081DE748117882F4460429728AD0CFFFB425014E868A13BD56FB6657D81D268191BDAD3DC09775C1D3CA48CF62C5A8191BD65BD0BA3038C0950A5D36C8A9BA7A39EFB766EC990983EF5C0329BA3038C0950A5D36D5E8D9A59859A8B665C1D6FB8A26C38176E601842F6C81A1F004C90652538430CDED94BCBF13EF3B93EC92FD9297F6718AA50765F7900637BC468E7E89D8C5D6A7F4EDE966BC389F395957E7521B51C24C7702A67D5C33162DBA43225CD8A89F9FFED5BD9FB417555E1C53F199C2BB95B5C8C57E37DE458B4C7702A67D5C3316FA3894348FB808DB985633C00BAEBE4F574AF45C6390F7469DAA53EE0834AAEE X-B7AD71C0: 14C14B24D00AF5AC321EF223B8115265C69B993890792DF82CDD5689AFBDA7A24A6D60772A99906F8E1CD14B953EB46D2F93436E023BC066355D89D7DBCDD132 X-C1DE0DAB: 0D63561A33F958A53E68B10B88E5CA58B8F5A663843A3D41813DAB78510F0C8ED59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75968C9853642EB7C3410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D342833AC5E8E9ACF1CA6D227FBADACF43F28F5E89E10D11400720BEAF432367164106F17C74DFA66991D7E09C32AA3244C092052BD28B704F1D3F3B69EA04ADE85F94338140B71B8EEFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojvz1c9SWJtj+4UOmxb5UNhg== X-Mailru-Sender: 504CC1E875BF3E7D9BC0E5172ADA3110693A3518FBB8F81AFE7F6E1FAB425C01380230E2385EB02707784C02288277CA03E0582D3806FB6A5317862B1921BA260ED6CFD6382C13A6112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH vshard 10/11] sched: introduce vshard.storage.sched module 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: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Thanks for the review! On 24.02.2021 11:28, Oleg Babin wrote: > Thanks for you patch. It's a brief review - I hope I'll look once again on this patch. > > Consider 2 comments below. > > >> diff --git a/vshard/storage/ref.lua b/vshard/storage/ref.lua >> index 7589cb9..2daad6b 100644 >> --- a/vshard/storage/ref.lua >> +++ b/vshard/storage/ref.lua >> @@ -341,6 +358,14 @@ local function ref_del(rid, sid) >>       return session:del(rid) >>   end >>   +local function ref_next_deadline() >> +    local session = M.session_heap:top() >> +    if not session then >> +        return fiber_clock() + TIMEOUT_INFINITY >> +    end > > Does it make sence? inf + fiber_clock() = inf Indeed. I could simply return the infinite deadline. ==================== local fiber_clock = lfiber.clock local fiber_yield = lfiber.yield local DEADLINE_INFINITY = lconsts.DEADLINE_INFINITY -local TIMEOUT_INFINITY = lconsts.TIMEOUT_INFINITY local LUA_CHUNK_SIZE = lconsts.LUA_CHUNK_SIZE ==================== local function ref_next_deadline() local session = M.session_heap:top() - if not session then - return fiber_clock() + TIMEOUT_INFINITY - end - return session.deadline + return session and session.deadline or DEADLINE_INFINITY end ==================== >> diff --git a/vshard/storage/sched.lua b/vshard/storage/sched.lua >> new file mode 100644 >> index 0000000..0ac71f4 >> --- /dev/null >> +++ b/vshard/storage/sched.lua >> @@ -0,0 +1,231 @@ >> +local function sched_wait_anything(timeout) >> +    return fiber_cond_wait(M.cond, timeout) >> +end >> + >> +-- >> +-- Return the remaining timeout in case there was a yield. This helps to save >> +-- current clock get in the caller code if there were no yields. >> +-- >> +local function sched_ref_start(timeout) >> +    local deadline = fiber_clock() + timeout > > Let's do it after fast check to eliminate excess fiber_clock call. > > Also there are several similar places below. Please fix them as well. Good idea, fixed. However there are just 2 such places. ==================== @@ -79,13 +79,13 @@ end -- current clock get in the caller code if there were no yields. -- local function sched_ref_start(timeout) - local deadline = fiber_clock() + timeout - local ok, err + local deadline, ok, err -- Fast-path. Moves are extremely rare. No need to inc-dec the ref queue -- then nor try to start some loops. if M.move_count == 0 and M.move_queue == 0 then goto success end + deadline = fiber_clock() + timeout M.ref_queue = M.ref_queue + 1 @@ -132,8 +132,7 @@ end -- current clock get in the caller code if there were no yields. -- local function sched_move_start(timeout) - local deadline = fiber_clock() + timeout - local ok, err, ref_deadline + local ok, err, deadline, ref_deadline local lref = lregistry.storage_ref -- Fast-path. Refs are not extremely rare *when used*. But they are not -- expected to be used in a lot of installations. So most of the times the @@ -141,6 +140,7 @@ local function sched_move_start(timeout) if M.ref_count == 0 and M.ref_queue == 0 then goto success end + deadline = fiber_clock() + timeout M.move_queue = M.move_queue + 1 ==================== I also removed some debug code which I forgot first time: ==================== @@ -18,11 +18,6 @@ local small_timeout = 0.000001 -- rebalancer. -- -box.cfg{ - log = 'log.txt' -} --- io.write = function(...) require('log').info(...) end - -- ====================