Skip to content

Commit

Permalink
replicaset: properly compare table URIs
Browse files Browse the repository at this point in the history
URI comparison is used in order not to recreate netbox connections
on storage/router reconfig. For tables it wasn't working properly
when compared the tables by values. The patch introduces deep
comparison. For example, now {3313} and 3313 are considered the
same URI. If such a change happened in vshard config, it won't
lead to a reconnect.

Closes #325
  • Loading branch information
Gerold103 committed May 10, 2022
1 parent c97a37d commit 5b2495f
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 1 deletion.
33 changes: 33 additions & 0 deletions test/router-luatest/router_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,39 @@ g.test_map_callrw_raw = function(g)
end)
end

g.test_uri_compare_and_reuse = function(g)
-- Multilisten itself is not used, but URI-table is supported since the same
-- version.
t.run_only_if(vutil.feature.multilisten)

local rs1_uuid = g.replica_1_a:replicaset_uuid()
local rep_1_a_uuid = g.replica_1_a:instance_uuid()
local bid = vtest.storage_first_bucket(g.replica_1_a)
local res, err

local new_cfg = vtest.config_new(cfg_template)
local rs_1_cfg = new_cfg.sharding[rs1_uuid]
local rep_1_a_cfg = rs_1_cfg.replicas[rep_1_a_uuid]
t.assert_equals(type(rep_1_a_cfg.uri), 'string', 'URI is a string')

-- Set a key in the session to check later for a reconnect.
res, err = g.router:exec(callrw_session_set, {bid, 1, 10, wait_timeout})
t.assert_equals(err, nil, 'no error')
t.assert(res, 'set session key')

-- Make the URI a table but it is still the same.
rep_1_a_cfg.uri = {rep_1_a_cfg.uri}
vtest.router_cfg(g.router, new_cfg)

-- The connection is still the same - session key remains.
res, err = g.router:exec(callrw_session_get, {bid, 1, wait_timeout})
t.assert_equals(err, nil, 'no error')
t.assert_equals(res, 10, 'get session key')

-- Restore the globals back.
vtest.router_cfg(g.router, cluster_cfg)
end

g.test_multilisten = function(g)
t.run_only_if(vutil.feature.multilisten)

Expand Down
83 changes: 83 additions & 0 deletions test/unit-luatest/util_test.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
local t = require('luatest')
local uuid = require('uuid')
local util = require('vshard.util')

local g = t.group('util')

g.test_uri_eq = function()
local uri_eq = util.uri_eq
--
-- Equal.
--
t.assert(uri_eq(1, 1))
t.assert(uri_eq('1', '1'))
t.assert(uri_eq({1}, 1))
t.assert(uri_eq(1, {1}))
t.assert(uri_eq('1', {'1'}))
t.assert(uri_eq({'1'}, '1'))
t.assert(uri_eq({'1'}, {'1'}))
t.assert(uri_eq({1}, {1}))

t.assert(uri_eq(1, '1'))
t.assert(uri_eq('1', 1))
t.assert(uri_eq({'1'}, 1))
t.assert(uri_eq('1', {1}))
t.assert(uri_eq({1}, '1'))
t.assert(uri_eq(1, {'1'}))
t.assert(uri_eq({1}, {'1'}))
t.assert(uri_eq({'1'}, {1}))

t.assert(uri_eq({
1, params = {key1 = 10, key2 = 20}
}, {
1, params = {key1 = 10, key2 = 20}
}))

t.assert(uri_eq({
'1', params = {key1 = 10, key2 = '20'}
}, {
1, params = {key1 = 10, key2 = 20}
}))

t.assert(uri_eq({
1, params = {key1 = {'10', 20}, key2 = 30}
}, {
'1', params = {key1 = {10, 20}, key2 = 30}
}))

t.assert(uri_eq(
'localhost:1?key1=10&key1=20&key2=30',
{'localhost:1', params = {key1 = {10, 20}, key2 = 30}}
))
--
-- Not equal.
--
t.assert(not uri_eq(1, 2))
t.assert(not uri_eq('1', '2'))

t.assert(not uri_eq({
1, params = {key1 = 10, key2 = 20}
}, {
1, params = {key1 = 20, key2 = 10}
}))

t.assert(not uri_eq({
'1', params = {key1 = 10}
}, {
1, params = {key1 = 10, key2 = 20}
}))

t.assert(not uri_eq({
1, params = {key1 = {10}, key2 = 30}
}, {
1, params = {key1 = {10, 20}, key2 = 30}
}))

t.assert(not uri_eq(
'localhost:1?key1=10&key1=20&key2=30',
{'localhost:1', params = {key1 = {20, 10}, key2 = 30}}
))

t.assert_error(uri_eq, 1, uuid.new())
t.assert_error(uri_eq, nil, 1)
end
2 changes: 1 addition & 1 deletion vshard/replicaset.lua
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ local function rebind_replicasets(replicasets, old_replicasets)
for replica_uuid, replica in pairs(replicaset.replicas) do
local old_replica = old_replicaset and
old_replicaset.replicas[replica_uuid]
if old_replica and old_replica.uri == replica.uri then
if old_replica and util.uri_eq(old_replica.uri, replica.uri) then
local conn = old_replica.conn
replica.conn = conn
replica.down_ts = old_replica.down_ts
Expand Down
38 changes: 38 additions & 0 deletions vshard/util.lua
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,43 @@ end

local tnt_version = lversion.parse(_TARANTOOL)

--
-- Deep comparison of tables. Stolen from built-in table.equals() which sadly is
-- not available in 1.10.
--
local function table_equals(a, b)
if type(a) ~= 'table' or type(b) ~= 'table' then
return type(a) == type(b) and a == b
end
local mta = getmetatable(a)
local mtb = getmetatable(b)
if mta and mta.__eq or mtb and mtb.__eq then
return a == b
end
for k, v in pairs(a) do
if not table_equals(v, b[k]) then
return false
end
end
for k, _ in pairs(b) do
if type(a[k]) == 'nil' then
return false
end
end
return true
end

local function uri_eq(a, b)
a = luri.parse(a)
b = luri.parse(b)
-- Query is inconsistent: a string URI with query params and a table URI
-- with explicitly specified same params are parsed into tables whose
-- queries are different. Discard them.
a.query = nil
b.query = nil
return table_equals(a, b)
end

--
-- Extract parts of a tuple.
-- @param tuple Tuple to extract a key from.
Expand Down Expand Up @@ -243,6 +280,7 @@ local feature = {
}

return {
uri_eq = uri_eq,
tuple_extract_key = tuple_extract_key,
reloadable_fiber_create = reloadable_fiber_create,
generate_self_checker = generate_self_checker,
Expand Down

0 comments on commit 5b2495f

Please sign in to comment.