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 F1F276C1BE; Wed, 10 Feb 2021 11:58:13 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org F1F276C1BE DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1612947494; bh=HKLRAsDcD47a8Uv67vvVM6dBshmGQ7oi9Wz+YSvRDxo=; 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=u70QA8KKdehsMJRAfaWYI40qSZsvjkON+qv7toxUHcZMaB7mwNBuZTulR/1iaoUTY h+s8YhLD8xQtY5kACkg8xpi10vSPdKcRnBTAZRXPoPlrkm08ITVmcJnzRelYVyzX3e G4P14w1FVhVtKY0cGuzIGOhrwZBFFAgWS7AhVMEs= Received: from smtp53.i.mail.ru (smtp53.i.mail.ru [94.100.177.113]) (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 9A9D66C1BE for ; Wed, 10 Feb 2021 11:57:47 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 9A9D66C1BE Received: by smtp53.i.mail.ru with esmtpa (envelope-from ) id 1l9lJf-0003A9-2b; Wed, 10 Feb 2021 11:57:43 +0300 To: Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org, yaroslav.dynnikov@tarantool.org References: <0898352fb536f41625736e7a1a60bc0e83cc3379.1612914070.git.v.shpilevoy@tarantool.org> Message-ID: <638701c4-6e5d-b5e7-5db8-9e7baf7ee7a8@tarantool.org> Date: Wed, 10 Feb 2021 11:57:41 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: <0898352fb536f41625736e7a1a60bc0e83cc3379.1612914070.git.v.shpilevoy@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD953AC099BC0052A9C6D5758EA387698D8F2F71171D5C2C2E5182A05F5380850402614A7CE917ECBCC307D17E4E69721C69BE0E49BF326D92E2537499C9E098D11 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE73870E1FF9A049D91EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637DB576DCB83B448D28638F802B75D45FF5571747095F342E8C7A0BC55FA0FE5FC8767A9B625D33EB46AF1322E3DBDB4DF8832D24F853A589B389733CBF5DBD5E913377AFFFEAFD269A417C69337E82CC2CC7F00164DA146DAFE8445B8C89999729449624AB7ADAF37F6B57BC7E64490611E7FA7ABCAF51C92176DF2183F8FC7C07E7E81EEA8A9722B8941B15DA834481F9449624AB7ADAF37BA3038C0950A5D3613377AFFFEAFD2691661749BA6B977350AC927CACCC122537B076A6E789B0E97A8DF7F3B2552694A1E7802607F20496D49FD398EE364050FD76C6ED7039589DE8FBB52F5C7ECD1BBB3661434B16C20AC78D18283394535A975ECD9A6C639B01BC09775C1D3CA48CF704239268B1C12EC35872C767BF85DA22EF20D2F80756B5F40A5AABA2AD3711975ECD9A6C639B01B78DA827A17800CE70740AD75FEDF3C08731C566533BA786A40A5AABA2AD371193C9F3DD0FB1AF5EB82E77451A5C57BD33C9F3DD0FB1AF5EB4E70A05D1297E1BBCB5012B2E24CD356 X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A24A6D60772A99906F8E1CD14B953EB46D57C8690349858B4B355D89D7DBCDD132 X-C1DE0DAB: 0D63561A33F958A5536C4D658330936EE7C5F79E8051B94F2492A669239841FFD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75F04B387B5D7535DE410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34A503FBFE8BE8FC498666B369379DFE29D80D1CDDBF3DA3F7FAFB151A717B40A5DDA279BCED6779EC1D7E09C32AA3244C169EF5EDBE9D1EBF3A61009D0978F2DC5A1673A01BA68E40FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojmMmQ+JvDeDFAfxJKtOWEiA== X-Mailru-Sender: E02534FE7B43636DC6DBEBE9641397949644BDB2FC6DDCAD82BBCC16C7BCF94FF267DDCB615D7F8423E75C7104EB1B885DEE61814008E47C7013064206BFB89F93956FB04BA385BE9437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH 2/9] Use fiber.clock() instead of .time() everywhere 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 except two nits: - Seems you need to put "Closes #246" - Tarantool has "clock" module. I suggest to use "fiber_clock()" instead of simple "clock" to avoid possible confusing. On 10/02/2021 02:46, Vladislav Shpilevoy wrote: > Fiber.time() returns real time. It is affected by time corrections > in the system, and can be not monotonic. > > The patch makes everything in vshard use fiber.clock() instead of > fiber.time(). Also fiber.clock function is saved as an upvalue for > all functions in all modules using it. This makes the code a bit > shorter and saves 1 indexing of 'fiber' table. > > The main reason - in the future map-reduce feature the current > time will be used quite often. In some places it probably will be > the slowest action (given how slow FFI can be when not compiled by > JIT). > > Needed for #147 > --- > test/failover/failover.result | 4 ++-- > test/failover/failover.test.lua | 4 ++-- > vshard/replicaset.lua | 13 +++++++------ > vshard/router/init.lua | 16 ++++++++-------- > vshard/storage/init.lua | 16 ++++++++-------- > 5 files changed, 27 insertions(+), 26 deletions(-) > > diff --git a/test/failover/failover.result b/test/failover/failover.result > index 452694c..bae57fa 100644 > --- a/test/failover/failover.result > +++ b/test/failover/failover.result > @@ -261,13 +261,13 @@ test_run:cmd('start server box_1_d') > --- > - true > ... > -ts1 = fiber.time() > +ts1 = fiber.clock() > --- > ... > while rs1.replica.name ~= 'box_1_d' do fiber.sleep(0.1) end > --- > ... > -ts2 = fiber.time() > +ts2 = fiber.clock() > --- > ... > ts2 - ts1 < vshard.consts.FAILOVER_UP_TIMEOUT > diff --git a/test/failover/failover.test.lua b/test/failover/failover.test.lua > index 13c517b..a969e0e 100644 > --- a/test/failover/failover.test.lua > +++ b/test/failover/failover.test.lua > @@ -109,9 +109,9 @@ test_run:switch('router_1') > -- Revive the best replica. A router must reconnect to it in > -- FAILOVER_UP_TIMEOUT seconds. > test_run:cmd('start server box_1_d') > -ts1 = fiber.time() > +ts1 = fiber.clock() > while rs1.replica.name ~= 'box_1_d' do fiber.sleep(0.1) end > -ts2 = fiber.time() > +ts2 = fiber.clock() > ts2 - ts1 < vshard.consts.FAILOVER_UP_TIMEOUT > test_run:grep_log('router_1', 'New replica box_1_d%(storage%@') > > diff --git a/vshard/replicaset.lua b/vshard/replicaset.lua > index b13d05e..a74c0f8 100644 > --- a/vshard/replicaset.lua > +++ b/vshard/replicaset.lua > @@ -54,6 +54,7 @@ local luri = require('uri') > local luuid = require('uuid') > local ffi = require('ffi') > local util = require('vshard.util') > +local clock = fiber.clock > local gsc = util.generate_self_checker > > -- > @@ -88,7 +89,7 @@ local function netbox_on_connect(conn) > -- biggest priority. Really, it is not neccessary to > -- increase replica connection priority, if the current > -- one already has the biggest priority. (See failover_f). > - rs.replica_up_ts = fiber.time() > + rs.replica_up_ts = clock() > end > end > > @@ -100,7 +101,7 @@ local function netbox_on_disconnect(conn) > assert(conn.replica) > -- Replica is down - remember this time to decrease replica > -- priority after FAILOVER_DOWN_TIMEOUT seconds. > - conn.replica.down_ts = fiber.time() > + conn.replica.down_ts = clock() > end > > -- > @@ -174,7 +175,7 @@ local function replicaset_up_replica_priority(replicaset) > local old_replica = replicaset.replica > if old_replica == replicaset.priority_list[1] and > old_replica:is_connected() then > - replicaset.replica_up_ts = fiber.time() > + replicaset.replica_up_ts = clock() > return > end > for _, replica in pairs(replicaset.priority_list) do > @@ -403,7 +404,7 @@ local function replicaset_template_multicallro(prefer_replica, balance) > net_status, err = pcall(box.error, box.error.TIMEOUT) > return nil, lerror.make(err) > end > - local end_time = fiber.time() + timeout > + local end_time = clock() + timeout > while not net_status and timeout > 0 do > replica, err = pick_next_replica(replicaset) > if not replica then > @@ -412,7 +413,7 @@ local function replicaset_template_multicallro(prefer_replica, balance) > opts.timeout = timeout > net_status, storage_status, retval, err = > replica_call(replica, func, args, opts) > - timeout = end_time - fiber.time() > + timeout = end_time - clock() > if not net_status and not storage_status and > not can_retry_after_error(retval) then > -- There is no sense to retry LuaJit errors, such as > @@ -680,7 +681,7 @@ local function buildall(sharding_cfg) > else > zone_weights = {} > end > - local curr_ts = fiber.time() > + local curr_ts = clock() > for replicaset_uuid, replicaset in pairs(sharding_cfg.sharding) do > local new_replicaset = setmetatable({ > replicas = {}, > diff --git a/vshard/router/init.lua b/vshard/router/init.lua > index ba1f863..a530c29 100644 > --- a/vshard/router/init.lua > +++ b/vshard/router/init.lua > @@ -1,6 +1,7 @@ > local log = require('log') > local lfiber = require('fiber') > local table_new = require('table.new') > +local clock = lfiber.clock > > local MODULE_INTERNALS = '__module_vshard_router' > -- Reload requirements, in case this module is reloaded manually. > @@ -527,7 +528,7 @@ local function router_call_impl(router, bucket_id, mode, prefer_replica, > end > local timeout = opts.timeout or consts.CALL_TIMEOUT_MIN > local replicaset, err > - local tend = lfiber.time() + timeout > + local tend = clock() + timeout > if bucket_id > router.total_bucket_count or bucket_id <= 0 then > error('Bucket is unreachable: bucket id is out of range') > end > @@ -551,7 +552,7 @@ local function router_call_impl(router, bucket_id, mode, prefer_replica, > replicaset, err = bucket_resolve(router, bucket_id) > if replicaset then > ::replicaset_is_found:: > - opts.timeout = tend - lfiber.time() > + opts.timeout = tend - clock() > local storage_call_status, call_status, call_error = > replicaset[call](replicaset, 'vshard.storage.call', > {bucket_id, mode, func, args}, opts) > @@ -583,7 +584,7 @@ local function router_call_impl(router, bucket_id, mode, prefer_replica, > -- if reconfiguration had been started, > -- and while is not executed on router, > -- but already is executed on storages. > - while lfiber.time() <= tend do > + while clock() <= tend do > lfiber.sleep(0.05) > replicaset = router.replicasets[err.destination] > if replicaset then > @@ -598,7 +599,7 @@ local function router_call_impl(router, bucket_id, mode, prefer_replica, > -- case of broken cluster, when a bucket > -- is sent on two replicasets to each > -- other. > - if replicaset and lfiber.time() <= tend then > + if replicaset and clock() <= tend then > goto replicaset_is_found > end > end > @@ -623,7 +624,7 @@ local function router_call_impl(router, bucket_id, mode, prefer_replica, > end > end > lfiber.yield() > - until lfiber.time() > tend > + until clock() > tend > if err then > return nil, err > else > @@ -749,7 +750,7 @@ end > -- connections must be updated. > -- > local function failover_collect_to_update(router) > - local ts = lfiber.time() > + local ts = clock() > local uuid_to_update = {} > for uuid, rs in pairs(router.replicasets) do > if failover_need_down_priority(rs, ts) or > @@ -772,7 +773,7 @@ local function failover_step(router) > if #uuid_to_update == 0 then > return false > end > - local curr_ts = lfiber.time() > + local curr_ts = clock() > local replica_is_changed = false > for _, uuid in pairs(uuid_to_update) do > local rs = router.replicasets[uuid] > @@ -1230,7 +1231,6 @@ local function router_sync(router, timeout) > timeout = router.sync_timeout > end > local arg = {timeout} > - local clock = lfiber.clock > local deadline = timeout and (clock() + timeout) > local opts = {timeout = timeout} > for rs_uuid, replicaset in pairs(router.replicasets) do > diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua > index 1b48bf1..c7335fc 100644 > --- a/vshard/storage/init.lua > +++ b/vshard/storage/init.lua > @@ -5,6 +5,7 @@ local netbox = require('net.box') -- for net.box:self() > local trigger = require('internal.trigger') > local ffi = require('ffi') > local yaml_encode = require('yaml').encode > +local clock = lfiber.clock > > local MODULE_INTERNALS = '__module_vshard_storage' > -- Reload requirements, in case this module is reloaded manually. > @@ -695,7 +696,7 @@ local function sync(timeout) > log.debug("Synchronizing replicaset...") > timeout = timeout or M.sync_timeout > local vclock = box.info.vclock > - local tstart = lfiber.time() > + local tstart = clock() > repeat > local done = true > for _, replica in ipairs(box.info.replication) do > @@ -711,7 +712,7 @@ local function sync(timeout) > return true > end > lfiber.sleep(0.001) > - until not (lfiber.time() <= tstart + timeout) > + until not (clock() <= tstart + timeout) > log.warn("Timed out during synchronizing replicaset") > local ok, err = pcall(box.error, box.error.TIMEOUT) > return nil, lerror.make(err) > @@ -1280,10 +1281,9 @@ local function bucket_send_xc(bucket_id, destination, opts, exception_guard) > ref.rw_lock = true > exception_guard.ref = ref > exception_guard.drop_rw_lock = true > - local deadline = lfiber.clock() + (opts and opts.timeout or 10) > + local deadline = clock() + (opts and opts.timeout or 10) > while ref.rw ~= 0 do > - if not M.bucket_rw_lock_is_ready_cond:wait(deadline - > - lfiber.clock()) then > + if not M.bucket_rw_lock_is_ready_cond:wait(deadline - clock()) then > status, err = pcall(box.error, box.error.TIMEOUT) > return nil, lerror.make(err) > end > @@ -1579,7 +1579,7 @@ function gc_bucket_f() > -- specified time interval the buckets are deleted both from > -- this array and from _bucket space. > local buckets_for_redirect = {} > - local buckets_for_redirect_ts = lfiber.time() > + local buckets_for_redirect_ts = clock() > -- Empty sent buckets, updated after each step, and when > -- buckets_for_redirect is deleted, it gets empty_sent_buckets > -- for next deletion. > @@ -1614,7 +1614,7 @@ function gc_bucket_f() > end > end > > - if lfiber.time() - buckets_for_redirect_ts >= > + if clock() - buckets_for_redirect_ts >= > consts.BUCKET_SENT_GARBAGE_DELAY then > status, err = gc_bucket_drop(buckets_for_redirect, > consts.BUCKET.SENT) > @@ -1629,7 +1629,7 @@ function gc_bucket_f() > else > buckets_for_redirect = empty_sent_buckets or {} > empty_sent_buckets = nil > - buckets_for_redirect_ts = lfiber.time() > + buckets_for_redirect_ts = clock() > end > end > ::continue::