* [tarantool-patches] [PATCH V4] Prohibit router/storage configuration from different fibers
@ 2018-09-24 15:22 Evgeniy Zaikin
2018-10-03 14:50 ` [tarantool-patches] " Vladislav Shpilevoy
0 siblings, 1 reply; 2+ messages in thread
From: Evgeniy Zaikin @ 2018-09-24 15:22 UTC (permalink / raw)
To: tarantool-patches
Subject: [PATCH V4] Prohibit router/storage configuration from different fibers
Fixes: #140
---
Ticket:https://github.com/tarantool/vshard/issues/140
Branch:https://github.com/tarantool/vshard/tree/rave4life/gh-140-prohibit-multiple-cfg
Hi! Thank you for review. This is my next patch version - hope it is
correct. You'll find patch below.
On Thu, 20 Sep 2018 at 22:59, Vladislav Shpilevoy
<v.shpilevoy@tarantool.org> wrote:
>
> Hi! Sorry for the last response - I was on vacation last
> 2 weeks. See 8 comments below.
>
> On 11/09/2018 12:32, Evgeniy Zaikin wrote:
> > From 5ce83d5db595455a14a58aedf9e6411f5adf1d7c Mon Sep 17 00:00:00 2001
> > From: rave4life <evgen4rave@gmail.com>
> > Date: Fri, 31 Aug 2018 16:41:11 +0300
> > Subject: [PATCH V3] Prohibit router/storage configuration from different fibers
> >
> > Fixes: #140
> > ---
> > Ticket:https://github.com/tarantool/vshard/issues/140
> > Branch:https://github.com/tarantool/vshard/tree/rave4life/gh-140-prohibit-multiple-cfg
> >
>
> > diff --git a/test/router/router.result b/test/router/router.result
> > index 000307d..ada8309 100644
> > --- a/test/router/router.result
> > +++ b/test/router/router.result
> > @@ -1140,6 +1140,45 @@ test_run:drop_cluster(REPLICASET_2)
> > while test_run:grep_log('router_1', 'disconnected from ') == nil do
> > fiber.sleep(0.1) end
> > ---
> > ...
> > +-- gh-140: prohibit vshard.router/storage.cfg from multiple fibers.
>
Fixed.
> 1. Sorry, still out of 80 symbols. It is a strict rule. Please,
> inspect your patch carefully for such code style things before
> sending to a review.
>
> > +cfg = {sharding = require('localcfg').sharding}
> > +---
> > +...
> > +vshard = require('vshard')
> > +---
> > +...
> > +fiber = require('fiber')
>
OK. Switched to 'storage_1_a' for storage and to 'router_1' for router.
> 2. Please, do not use 'default' server for testing
> storage or router. Router/router.test.lua starts multiple
> Tarantool instances: storage_1_a - storage_2_b and router_1
> (see the very beginning of the test file) and switches
> between them to test router or storage things. 'Default'
> server shall not be used for this. Please, use test_run:switch
> and router_1 server for a test. In such a case you will not
> have require('vshard') and require('fiber') since they are
> already loaded on router_1 instance.
>
> There is a reason why 'default' can not be used - once a
> router is started on an instance, it can not be stopped
> without restart (its internal fibers, I mean). For the storage
> things are worse - it creates new spaces, users, functions
> and we have no way to cleanup those things. So once 'default'
> is used for testing, it will affect next tests.
>
> For this problem we have an issue: https://github.com/tarantool/vshard/issues/121
> You can try it next after this patch is finished if you want.
>
> > +---
> > +...
> > +vshard.router.cfg(cfg)
> > +---
> > +...
> > +vshard.router.internal.errinj.ERRINJ_MULTIPLE_CFG = false
>
Set to 'nil' according to your suggestion (8-th comment)
> 3. It is false by default already (as I see in router/init.lua),
> so what is a point of this line? I removed it and the test
> passed ok. I guess it is a typo. Please, fix.
>
> > +---
> > +...
> > +c = fiber.channel(2)
> > +---
> > +...
> > diff --git a/test/storage/storage.result b/test/storage/storage.result
> > index e3e9e7f..0228445 100644
> > --- a/test/storage/storage.result
> > +++ b/test/storage/storage.result
> > @@ -688,6 +688,48 @@ rs:callro('echo', {'some_data'})
> > - null
> > - null
> > ...
> > +-- gh-140: prohibit vshard.router/storage.cfg from multiple fibers.
> > +cfg = {sharding = {}}
> > +---
> > +...
> > +cfg.sharding[box.info.cluster.uuid] = {replicas = {}}
> > +---
> > +...
> > +cfg.sharding[box.info.cluster.uuid].replicas[box.info.uuid] = {uri =
> > 'storage:storage@127.0.0.1:3309', name = 'test_storage', master =
> > true}
> > +---
> > +...
> > +vshard = require('vshard')
> > +---
> > +...
> > +fiber = require('fiber')
>
OK, removed.
> 4. For the same reason I explained above, you do not
> need 'require' here. This code is executed on 'storage_1_a' where
> both fiber and vshard are loaded already
> (see line storage.test.lua:137 - this is the latest switch before
> your test).
>
> > +---
> > +...
> > +vshard.storage.internal.errinj.ERRINJ_MULTIPLE_CFG = false
>
> 5. Same as 3.
>
> > +---
> > +...
> > +c = fiber.channel(2)
> > +---
> > +...
> > diff --git a/vshard/router/init.lua b/vshard/router/init.lua
> > index 7573801..446b1a0 100644
> > --- a/vshard/router/init.lua
> > +++ b/vshard/router/init.lua
> > @@ -28,6 +28,7 @@ if not M then
> > ---------------- Common module attributes ----------------
> > errinj = {
> > ERRINJ_CFG = false,
> > + ERRINJ_MULTIPLE_CFG = false,
> > ERRINJ_FAILOVER_CHANGE_CFG = false,
> > ERRINJ_RELOAD = false,
> > ERRINJ_LONG_DISCOVERY = false,
> > @@ -53,6 +54,10 @@ local ROUTER_TEMPLATE = {
> > name = nil,
> > -- The last passed configuration.
> > current_cfg = nil,
> > + -- Fiber channel as the locker to prevent multiple config.
> > + cfg_lock = nil,
> > + -- Configuration counter to control and verify locker.
> > + cfg_counter = 0,
>
OK, 'cfg_counter' is also removed.
> 6. Please, remove cfg_counter. Its value > 0 is unreachable
> during normal work of router/storage. If you need a counter for
> testing lock, you can use erring.ERRINJ_MULTIPLE_CFG as a counter.
> It is not necessary to make each errinj as a strict boolean. See
> my proposal below in the comment 8.
>
> > -- Time to outdate old objects on reload.
> > connection_outdate_delay = nil,
> > -- Bucket map cache.
> > @@ -594,6 +599,29 @@ local function router_cfg(router, cfg, is_reload)
> > end
> > end
> >
> > +--------------------------------------------------------------------------------
> > +-- Configuration wrapper
> > +--------------------------------------------------------------------------------
>
> 7. Function comment should look like this:
>
> --
> -- My function description wrapped by 66
> -- symbols.
> -- @param name1 Parameter 1 description.
> -- @param name2 Parameter 2 description.
> --
> -- [if returns anything]
> -- @retval Returned value description.
> -- @retval Next possible return value description.
> --
>
OK, comment was added.
> > +
> > +local function cfg_wrapper(router, ...)
> > + local enable_lock = not M.errinj.ERRINJ_MULTIPLE_CFG
> > + if enable_lock then
> > + router.cfg_lock:put(true)
> > + end
> > + if router.cfg_counter > 0 then
> > + error('Error: cfg_counter > 0')
> > + end
> > + router.cfg_counter = router.cfg_counter + 1
> > + local status, err = pcall(router_cfg, router, ...)
> > + router.cfg_counter = router.cfg_counter - 1
> > + if enable_lock then
> > + router.cfg_lock:get()
> > + end
> > + if not status then
> > + error(err)
> > + end
> > +end
>
> 8. Too complex. Lets use errinj.ERRINJ_MULTIPLE_CFG as
> an integer and set it to nil by default
>
> M = {
> ...
> errinj = {
> ...
> ERRINJ_MULTIPLE_CFG = nil
> }
> ...
> }
>
> ...
>
> function cfg_wrapper()
> if ERRINJ_MULTIPLE_CFG then
> ERRINJ_MULTIPLE_CFG = ERRINJ_MULTIPLE_CFG + 1
> if ERRINJ_MULTIPLE_CFG > 1 then
> error('Multiple cfgs at the same time')
> end
> status, err = pcall(router_cfg, router, ...)
> ERRINJ_MULTIPLE_CFG = ERRINJ_MULTIPLE_CFG - 1
> else
> lock:put(true)
> status, err = pcall(router_cfg, router, ...)
> lock:get()
> end
> if not status then
> error(err)
> end
> return
> end
>
>
> ERRINJ_MULTIPLE_CFG is nil by default and to active it
> you can do this in a test:
>
> ERRINJ_MULTIPLE_CFG = 0
>
> This will trigger counter in my pseudo code above since '0'
> in Lua is true.
>
Yes, placed new code instead.
So, my fixed patch is below.
Changes in v4:
- Checked code line length for 80 symbols
- Switched to appropriate Tarantool instances in test
- Error injection works now as a triggered counter
- Added comment to wrapper function
---
test/router/router.result | 38 +++++++++++++++++++++++++++++++++++
test/router/router.test.lua | 16 +++++++++++++++
test/storage/storage.result | 35 ++++++++++++++++++++++++++++++++
test/storage/storage.test.lua | 16 +++++++++++++++
vshard/router/init.lua | 38 ++++++++++++++++++++++++++++++++---
vshard/storage/init.lua | 34 +++++++++++++++++++++++++++++--
6 files changed, 172 insertions(+), 5 deletions(-)
diff --git a/test/router/router.result b/test/router/router.result
index 000307d..5ce48b1 100644
--- a/test/router/router.result
+++ b/test/router/router.result
@@ -1130,6 +1130,44 @@ vshard.router.static:route(1):callro('echo',
{'some_data'})
- null
- null
...
+-- gh-140: prohibit vshard.router/storage.cfg from multiple fibers.
+cfg = {sharding = require('localcfg').sharding}
+---
+...
+vshard.router.cfg(cfg)
+---
+...
+c = fiber.channel(2)
+---
+...
+vshard.router.internal.errinj.ERRINJ_MULTIPLE_CFG = 0
+---
+...
+f = function()
+ fiber.create(function()
+ local status, err = pcall(vshard.router.cfg, cfg)
+ c:put(status and true or false)
+ end)
+end
+---
+...
+f()
+---
+...
+f()
+---
+...
+c:get()
+---
+- true
+...
+c:get()
+---
+- true
+...
+vshard.router.internal.errinj.ERRINJ_MULTIPLE_CFG = nil
+---
+...
_ = test_run:switch("default")
---
...
diff --git a/test/router/router.test.lua b/test/router/router.test.lua
index 9f32d3e..5590f95 100644
--- a/test/router/router.test.lua
+++ b/test/router/router.test.lua
@@ -434,6 +434,22 @@ vshard.router.route(1):callro('echo', {'some_data'})
-- object.
vshard.router.static:route(1):callro('echo', {'some_data'})
+-- gh-140: prohibit vshard.router/storage.cfg from multiple fibers.
+cfg = {sharding = require('localcfg').sharding}
+vshard.router.cfg(cfg)
+vshard.router.internal.errinj.ERRINJ_MULTIPLE_CFG = 0
+c = fiber.channel(2)
+f = function()
+ fiber.create(function()
+ local status, err = pcall(vshard.router.cfg, cfg)
+ c:put(status and true or false)
+ end)
+end
+f()
+f()
+c:get()
+c:get()
+vshard.router.internal.errinj.ERRINJ_MULTIPLE_CFG = nil
_ = test_run:switch("default")
test_run:drop_cluster(REPLICASET_2)
diff --git a/test/storage/storage.result b/test/storage/storage.result
index e3e9e7f..b528b36 100644
--- a/test/storage/storage.result
+++ b/test/storage/storage.result
@@ -688,6 +688,41 @@ rs:callro('echo', {'some_data'})
- null
- null
...
+-- gh-140: prohibit storage.cfg from multiple fibers.
+_ = test_run:switch("storage_1_a")
+---
+...
+vshard.storage.internal.errinj.ERRINJ_MULTIPLE_CFG = 0
+---
+...
+c = fiber.channel(2)
+---
+...
+f = function()
+ fiber.create(function()
+ local status, err = pcall(vshard.storage.cfg, cfg, box.info.uuid)
+ c:put(status and true or false)
+ end)
+end
+---
+...
+f()
+---
+...
+f()
+---
+...
+c:get()
+---
+- true
+...
+c:get()
+---
+- true
+...
+vshard.storage.internal.errinj.ERRINJ_MULTIPLE_CFG = nil
+---
+...
_ = test_run:switch("default")
---
...
diff --git a/test/storage/storage.test.lua b/test/storage/storage.test.lua
index c07730b..12fff36 100644
--- a/test/storage/storage.test.lua
+++ b/test/storage/storage.test.lua
@@ -182,6 +182,22 @@ util.has_same_fields(old_internal, vshard.storage.internal)
_, rs = next(vshard.storage.internal.replicasets)
rs:callro('echo', {'some_data'})
+-- gh-140: prohibit vshard.router/storage.cfg from multiple fibers.
+_ = test_run:switch("storage_1_a")
+vshard.storage.internal.errinj.ERRINJ_MULTIPLE_CFG = 0
+c = fiber.channel(2)
+f = function()
+ fiber.create(function()
+ local status, err = pcall(vshard.storage.cfg, cfg, box.info.uuid)
+ c:put(status and true or false)
+ end)
+end
+f()
+f()
+c:get()
+c:get()
+vshard.storage.internal.errinj.ERRINJ_MULTIPLE_CFG = nil
+
_ = test_run:switch("default")
test_run:drop_cluster(REPLICASET_2)
test_run:drop_cluster(REPLICASET_1)
diff --git a/vshard/router/init.lua b/vshard/router/init.lua
index 7573801..93bef77 100644
--- a/vshard/router/init.lua
+++ b/vshard/router/init.lua
@@ -28,6 +28,7 @@ if not M then
---------------- Common module attributes ----------------
errinj = {
ERRINJ_CFG = false,
+ ERRINJ_MULTIPLE_CFG = nil,
ERRINJ_FAILOVER_CHANGE_CFG = false,
ERRINJ_RELOAD = false,
ERRINJ_LONG_DISCOVERY = false,
@@ -53,6 +54,8 @@ local ROUTER_TEMPLATE = {
name = nil,
-- The last passed configuration.
current_cfg = nil,
+ -- Fiber channel as the locker to prevent multiple config.
+ cfg_lock = nil,
-- Time to outdate old objects on reload.
connection_outdate_delay = nil,
-- Bucket map cache.
@@ -594,6 +597,34 @@ local function router_cfg(router, cfg, is_reload)
end
end
+--
+-- Wrapper around 'router_cfg' function to provide
+-- locking beafore / unlocking after pcall(router_cfg, router, ...),
+-- so it prevents 'router_cfg' been called from diffenrent fibers.
+-- @param router Router instance to be configured.
+-- @param ... Arguments which would be passed to the `router_cfg`.
+-- @retval true, ... on success
+--
+local function cfg_wrapper(router, ...)
+ local status, err
+ if M.errinj.ERRINJ_MULTIPLE_CFG then
+ M.errinj.ERRINJ_MULTIPLE_CFG = M.errinj.ERRINJ_MULTIPLE_CFG + 1
+ if M.errinj.ERRINJ_MULTIPLE_CFG > 1 then
+ error('Multiple cfgs at the same time')
+ end
+ status, err = pcall(router_cfg, router, ...)
+ M.errinj.ERRINJ_MULTIPLE_CFG = M.errinj.ERRINJ_MULTIPLE_CFG - 1
+ else
+ router.cfg_lock:put(true)
+ status, err = pcall(router_cfg, router, ...)
+ router.cfg_lock:get()
+ end
+ if not status then
+ error(err)
+ end
+ return true
+end
+
--------------------------------------------------------------------------------
-- Bootstrap
--------------------------------------------------------------------------------
@@ -862,7 +893,7 @@ end
local router_mt = {
__index = {
- cfg = function(router, cfg) return router_cfg(router, cfg, false) end,
+ cfg = function(router, cfg) return cfg_wrapper(router, cfg) end,
info = router_info;
buckets_info = router_buckets_info;
call = router_call;
@@ -920,6 +951,7 @@ local function router_new(name, cfg)
local router = table.deepcopy(ROUTER_TEMPLATE)
setmetatable(router, router_mt)
router.name = name
+ router.cfg_lock = lfiber.channel(1)
M.routers[name] = router
local ok, err = pcall(router_cfg, router, cfg)
if not ok then
@@ -936,7 +968,7 @@ end
local function legacy_cfg(cfg)
if M.static_router then
-- Reconfigure.
- router_cfg(M.static_router, cfg, false)
+ cfg_wrapper(M.static_router, cfg, false)
else
-- Create new static instance.
local router, err = router_new(STATIC_ROUTER_NAME, cfg)
@@ -961,7 +993,7 @@ if not rawget(_G, MODULE_INTERNALS) then
rawset(_G, MODULE_INTERNALS, M)
else
for _, router in pairs(M.routers) do
- router_cfg(router, router.current_cfg, true)
+ cfg_wrapper(router, router.current_cfg, true)
setmetatable(router, router_mt)
end
if M.static_router then
diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
index dc059ba..3b1b525 100644
--- a/vshard/storage/init.lua
+++ b/vshard/storage/init.lua
@@ -48,6 +48,8 @@ if not M then
---------------- Common module attributes ----------------
-- The last passed configuration.
current_cfg = nil,
+ -- Fiber channel as the locker to prevent multiple config.
+ cfg_lock = lfiber.channel(1),
--
-- All known replicasets used for bucket re-balancing.
-- See format in replicaset.lua.
@@ -66,6 +68,7 @@ if not M then
total_bucket_count = 0,
errinj = {
ERRINJ_CFG = false,
+ ERRINJ_MULTIPLE_CFG = nil,
ERRINJ_BUCKET_FIND_GARBAGE_DELAY = false,
ERRINJ_RELOAD = false,
ERRINJ_CFG_DELAY = false,
@@ -1843,6 +1846,33 @@ local function storage_cfg(cfg,
this_replica_uuid, is_reload)
collectgarbage()
end
+--
+-- Wrapper around 'storage_cfg' function to provide
+-- locking beafore / unlocking after pcall(storage_cfg, ...),
+-- so it prevents 'storage_cfg' been called from diffenrent fibers.
+-- @param ... Arguments which would be passed to the `storage_cfg`.
+-- @retval true, ... on success
+--
+local function storage_cfg_wrapper(...)
+ local status, err
+ if M.errinj.ERRINJ_MULTIPLE_CFG then
+ M.errinj.ERRINJ_MULTIPLE_CFG = M.errinj.ERRINJ_MULTIPLE_CFG + 1
+ if M.errinj.ERRINJ_MULTIPLE_CFG > 1 then
+ error('Multiple cfgs at the same time')
+ end
+ status, err = pcall(storage_cfg, ...)
+ M.errinj.ERRINJ_MULTIPLE_CFG = M.errinj.ERRINJ_MULTIPLE_CFG - 1
+ else
+ M.cfg_lock:put(true)
+ status, err = pcall(storage_cfg, ...)
+ M.cfg_lock:get()
+ end
+ if not status then
+ error(err)
+ end
+ return true
+end
+
--------------------------------------------------------------------------------
-- Monitoring
--------------------------------------------------------------------------------
@@ -2060,7 +2090,7 @@ if not rawget(_G, MODULE_INTERNALS) then
rawset(_G, MODULE_INTERNALS, M)
else
reload_evolution.upgrade(M)
- storage_cfg(M.current_cfg, M.this_replica.uuid, true)
+ storage_cfg_wrapper(M.current_cfg, M.this_replica.uuid, true)
M.module_version = M.module_version + 1
end
@@ -2103,7 +2133,7 @@ return {
rebalancing_is_in_progress = rebalancing_is_in_progress,
recovery_wakeup = recovery_wakeup,
call = storage_call,
- cfg = function(cfg, uuid) return storage_cfg(cfg, uuid, false) end,
+ cfg = function(cfg, uuid) return storage_cfg_wrapper(cfg, uuid, false) end,
info = storage_info,
buckets_info = storage_buckets_info,
buckets_count = storage_buckets_count,
--
2.17.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* [tarantool-patches] Re: [PATCH V4] Prohibit router/storage configuration from different fibers
2018-09-24 15:22 [tarantool-patches] [PATCH V4] Prohibit router/storage configuration from different fibers Evgeniy Zaikin
@ 2018-10-03 14:50 ` Vladislav Shpilevoy
0 siblings, 0 replies; 2+ messages in thread
From: Vladislav Shpilevoy @ 2018-10-03 14:50 UTC (permalink / raw)
To: tarantool-patches, Evgeniy Zaikin
Hi! Thanks for the fixes! See 3 comments below.
> diff --git a/test/router/router.result b/test/router/router.result
> index 000307d..5ce48b1 100644
> --- a/test/router/router.result
> +++ b/test/router/router.result
> @@ -1130,6 +1130,44 @@ vshard.router.static:route(1):callro('echo',
> {'some_data'})
> - null
> - null
> ...
1. Please, do not hurry. Looks like you did not run a test.
This is what I got for router/router.test.lua:
f = function()
---
- error: '[string "f = function() "]:1: ''end'' expected near ''<eof>'''
...
fiber.create(function()
---
- error: '[string " fiber.create(function() "]:1: ''end'' expected near ''<eof>'''
...
local status, err = pcall(vshard.router.cfg, cfg)
---
...
c:put(status and true or false)
---
- error: '[string "return c:put(status and true or false) "]:1: variable ''status''
is not declared'
...
end)
---
- error: '[string " end) "]:1: ''<eof>'' expected near ''end'''
...
end
---
- error: '[string "end "]:1: ''<eof>'' expected near ''end'''
...
f()
---
- error: '[string "return f() "]:1: variable ''f'' is not declared'
...
f()
---
- error: '[string "return f() "]:1: variable ''f'' is not declared'
...
c:get()
Obviously, your test is not about Lua syntax.
The error is trivial so I just inlined multiline functions, but got more errors
like this:
[001] vshard.storage.cfg(cfg, util.name_to_uuid.storage_1_a)
[001] ---
[001] +- true
[001] ...
Looks like you started to return a boolean from storage.cfg.
Please, don't. It changes behavior of a public API with no
reason.
This error is trivial as well, but the next is extra strange
and the same as in the previous review: I have removed this line
vshard.router.internal.errinj.ERRINJ_MULTIPLE_CFG = 0
And got no errors. So the test passes regardless of ERRINJ_MULTIPLE_CFG
value. Why? It shall not be so. Such a test makes no sense if it
tests nothing.
I understand your wish to push the patch as soon as possible, but
please, be more attentive. Elaborate each line of your code before
sending it to the mailing list. Run the entire test suite 3-4 times.
> +-- gh-140: prohibit vshard.router/storage.cfg from multiple fibers.
> +cfg = {sharding = require('localcfg').sharding}
> +---
> +...
> +vshard.router.cfg(cfg)
> +---
> +...
> +c = fiber.channel(2)
> +---
> +...
> +vshard.router.internal.errinj.ERRINJ_MULTIPLE_CFG = 0
> +---
> +...
> +f = function()
> + fiber.create(function()
> + local status, err = pcall(vshard.router.cfg, cfg)
> + c:put(status and true or false)
2. 'status' is boolean by definition of pcall() function. So
here you do <x and true or false>. Then look into this:
https://en.wikipedia.org/wiki/Boolean_algebra and use monotone
laws:
x and true = x
x or false = x
Apply it to your expression:
status and true or false = (status and true) or false =
= status or false = status
So just do:
c:put(status)
What is more, you can do this:
c:put((pcall(vshard.router.cfg, cfg))) - here I
wrapped pcall() into additional () to cut a second
retval off - this is nice Lua feature.
All the same comments for storage.test.lua.
> diff --git a/vshard/router/init.lua b/vshard/router/init.lua
> index 7573801..93bef77 100644
> --- a/vshard/router/init.lua
> +++ b/vshard/router/init.lua
> @@ -28,6 +28,7 @@ if not M then
> ---------------- Common module attributes ----------------
> errinj = {
> ERRINJ_CFG = false,
> + ERRINJ_MULTIPLE_CFG = nil,
> ERRINJ_FAILOVER_CHANGE_CFG = false,
> ERRINJ_RELOAD = false,
> ERRINJ_LONG_DISCOVERY = false,
> @@ -53,6 +54,8 @@ local ROUTER_TEMPLATE = {
> name = nil,
> -- The last passed configuration.
> current_cfg = nil,
> + -- Fiber channel as the locker to prevent multiple config.
> + cfg_lock = nil,
> -- Time to outdate old objects on reload.
> connection_outdate_delay = nil,
> -- Bucket map cache.
> @@ -594,6 +597,34 @@ local function router_cfg(router, cfg, is_reload)
> end
> end
>
> +--
> +-- Wrapper around 'router_cfg' function to provide
> +-- locking beafore / unlocking after pcall(router_cfg, router, ...),
> +-- so it prevents 'router_cfg' been called from diffenrent fibers.
3. Typos: 'beafore', 'diffenrent'. The same for storage cfg wrapper.
> +-- @param router Router instance to be configured.
> +-- @param ... Arguments which would be passed to the `router_cfg`.
> +-- @retval true, ... on success
> +--
> +local function cfg_wrapper(router, ...)
> + local status, err
> + if M.errinj.ERRINJ_MULTIPLE_CFG then
> + M.errinj.ERRINJ_MULTIPLE_CFG = M.errinj.ERRINJ_MULTIPLE_CFG + 1
> + if M.errinj.ERRINJ_MULTIPLE_CFG > 1 then
> + error('Multiple cfgs at the same time')
This error should appear in a test.
> + end
> + status, err = pcall(router_cfg, router, ...)
> + M.errinj.ERRINJ_MULTIPLE_CFG = M.errinj.ERRINJ_MULTIPLE_CFG - 1
> + else
> + router.cfg_lock:put(true)
> + status, err = pcall(router_cfg, router, ...)
> + router.cfg_lock:get()
> + end
> + if not status then
> + error(err)
> + end
> + return true
> +end
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-10-03 14:50 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-24 15:22 [tarantool-patches] [PATCH V4] Prohibit router/storage configuration from different fibers Evgeniy Zaikin
2018-10-03 14:50 ` [tarantool-patches] " Vladislav Shpilevoy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox