Skip to content

Commit

Permalink
replicaset: validate name in locate_master
Browse files Browse the repository at this point in the history
Currently, the name validation is not used, when locate_master()
is called. It makes an explicit call via the connection to obtain a
future object in order to figure out, whether the node is a master.

We should not make any calls to a replica until the time, we definitely
know, that its name and uuid was validated. Let's use replica_call
instead of conn.call.

Follow-up tarantool#426

NO_DOC=bugfix
  • Loading branch information
Serpentian committed Jan 26, 2024
1 parent bc84c65 commit c56639f
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 10 deletions.
27 changes: 27 additions & 0 deletions test/replicaset-luatest/vconnect_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,30 @@ test_group.test_async_no_yield = function(g)
ivshard.storage._call = _G._call
end)
end

--
-- Test, that during master search name is validated.
--
test_group.test_locate_master = function(g)
-- Replace name with the bad one.
local new_cfg = table.deepcopy(global_cfg)
local cfg_rs = new_cfg.sharding.replicaset
cfg_rs.replicas.bad = cfg_rs.replicas.replica
cfg_rs.replicas.replica = nil
local _, rs = next(vreplicaset.buildall(new_cfg))

-- Avoid noop in locate_master.
rs.master = nil
rs.is_master_auto = true
-- Retry, until the connection is established and
-- name mismach error is encountered.
local ok, is_nop, last_err
t.helpers.retrying(timeout_opts, function()
ok, is_nop, last_err = rs:locate_master()
t.assert_not_equals(last_err, nil)
end)

t.assert_equals(last_err.name, 'INSTANCE_NAME_MISMATCH')
t.assert_equals(is_nop, false)
t.assert_equals(ok, false)
end
28 changes: 18 additions & 10 deletions vshard/replicaset.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,7 @@ local function replicaset_locate_master(replicaset)
local func = 'vshard.storage._call'
local args = {'info'}
local const_timeout = consts.MASTER_SEARCH_TIMEOUT
local ok, res, err, f
local ok, res, err
local master = replicaset.master
if master then
local sync_opts = {timeout = const_timeout}
Expand Down Expand Up @@ -1091,17 +1091,20 @@ local function replicaset_locate_master(replicaset)
local futures = {}
local timeout = const_timeout
local deadline = fiber_clock() + timeout
local async_opts = {is_async = true}
local async_opts = {is_async = true, timeout = timeout}
local replicaset_id = replicaset.id
for replica_id, replica in pairs(replicaset.replicas) do
local conn = replicaset_connect_to_replica(replicaset, replica)
if conn:is_connected() then
ok, f = pcall(conn.call, conn, func, args, async_opts)
if not ok then
last_err = lerror.make(f)
replicaset_connect_to_replica(replicaset, replica)
ok, err = replica:is_connected()
if ok then
ok, res, err = replica_call(replica, func, args, async_opts)
if not ok and err ~= nil then
last_err = err
else
futures[replica_id] = f
futures[replica_id] = res
end
elseif err ~= nil then
last_err = err
end
end
local master_id
Expand Down Expand Up @@ -1198,8 +1201,13 @@ replicaset_mt.__index = index
local replica_mt = {
__index = {
is_connected = function(replica)
return replica.conn and replica.conn:is_connected() and
conn_vconnect_check_or_close(replica.conn)
if not replica.conn then
return false
end
if not replica.conn:is_connected() then
return false
end
return conn_vconnect_check_or_close(replica.conn)
end,
safe_uri = function(replica)
local uri = luri.parse(replica.uri)
Expand Down

0 comments on commit c56639f

Please sign in to comment.