[Tarantool-patches] [PATCH vshard 10/11] sched: introduce vshard.storage.sched module
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Thu Feb 25 00:50:02 MSK 2021
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
-
--
====================
More information about the Tarantool-patches
mailing list