<div dir="ltr"><div>Hi!</div><div><br></div><div>Thanks for the fix. LGTM.<br></div><div><br></div><div><div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><span><div><div dir="ltr">Best regards</div><div>Yaroslav Dynnikov<br></div></div></span></div></div></div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 7 Aug 2020 at 00:48, Vladislav Shpilevoy <<a href="mailto:v.shpilevoy@tarantool.org">v.shpilevoy@tarantool.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Reconfiguration in router and storage always creates new<br>
replica objects, and outdates old objects. Since most of the<br>
time reconfiguration does not change cluster topology and URIs,<br>
old connections are relocated to the new replicaset objects so as<br>
not to reconnect on each router.cfg() and storage.cfg().<br>
<br>
Connections were moved from old objects to new ones by UUID match<br>
not taking into account possible URI change.<br>
<br>
If URI was changed, the new replica object took the old connection<br>
with old URI, and kept trying to connect to the old address.<br>
<br>
The patch makes connection relocation work only if old and new URI<br>
match.<br>
<br>
Closes #245<br>
---<br>
Branch: <a href="http://github.com/tarantool/vshard/tree/gerold103/gh-245-router-reconnect" rel="noreferrer" target="_blank">http://github.com/tarantool/vshard/tree/gerold103/gh-245-router-reconnect</a><br>
Issue: <a href="https://github.com/tarantool/vshard/issues/245" rel="noreferrer" target="_blank">https://github.com/tarantool/vshard/issues/245</a><br>
<br>
test/router/reconnect_to_master.result | 55 ++++++++++++++++++++++++<br>
test/router/reconnect_to_master.test.lua | 26 +++++++++++<br>
vshard/replicaset.lua | 2 +-<br>
3 files changed, 82 insertions(+), 1 deletion(-)<br>
<br>
diff --git a/test/router/reconnect_to_master.result b/test/router/reconnect_to_master.result<br>
index d640f0b..aa33f9c 100644<br>
--- a/test/router/reconnect_to_master.result<br>
+++ b/test/router/reconnect_to_master.result<br>
@@ -169,6 +169,61 @@ is_disconnected()<br>
---<br>
- false<br>
...<br>
+--<br>
+-- gh-245: dynamic uri reconfiguration didn't work - even if URI was changed in<br>
+-- the config for any instance, it used old connection, because reconfiguration<br>
+-- compared connections by UUID instead of URI.<br>
+--<br>
+util = require('util')<br>
+---<br>
+...<br>
+-- Firstly, clean router from storage_1_a connection.<br>
+rs1_uuid = util.replicasets[1]<br>
+---<br>
+...<br>
+rs1_cfg = cfg.sharding[rs1_uuid]<br>
+---<br>
+...<br>
+cfg.sharding[rs1_uuid] = nil<br>
+---<br>
+...<br>
+vshard.router.cfg(cfg)<br>
+---<br>
+...<br>
+-- Now break the URI in the config.<br>
+old_uri = rs1_cfg.replicas[util.name_to_uuid.storage_1_a].uri<br>
+---<br>
+...<br>
+rs1_cfg.replicas[util.name_to_uuid.storage_1_a].uri = '<a href="https://bad_uri.com:123" rel="noreferrer" target="_blank">https://bad_uri.com:123</a>'<br>
+---<br>
+...<br>
+-- Apply the bad config.<br>
+cfg.sharding[rs1_uuid] = rs1_cfg<br>
+---<br>
+...<br>
+vshard.router.cfg(cfg)<br>
+---<br>
+...<br>
+-- Should fail - master is not available because of the bad URI.<br>
+res, err = vshard.router.callrw(1, 'echo', {1})<br>
+---<br>
+...<br>
+res == nil and err ~= nil<br>
+---<br>
+- true<br>
+...<br>
+-- Repair the config.<br>
+rs1_cfg.replicas[util.name_to_uuid.storage_1_a].uri = old_uri<br>
+---<br>
+...<br>
+vshard.router.cfg(cfg)<br>
+---<br>
+...<br>
+-- Should drop the old connection object and connect fine.<br>
+vshard.router.callrw(1, 'echo', {1})<br>
+---<br>
+- 1<br>
+...<br>
_ = test_run:switch("default")<br>
---<br>
...<br>
diff --git a/test/router/reconnect_to_master.test.lua b/test/router/reconnect_to_master.test.lua<br>
index c315c9f..87270af 100644<br>
--- a/test/router/reconnect_to_master.test.lua<br>
+++ b/test/router/reconnect_to_master.test.lua<br>
@@ -70,6 +70,32 @@ while is_disconnected() and i < max_iters do i = i + 1 fiber.sleep(0.1) end<br>
-- Master connection is active again.<br>
is_disconnected()<br>
<br>
+--<br>
+-- gh-245: dynamic uri reconfiguration didn't work - even if URI was changed in<br>
+-- the config for any instance, it used old connection, because reconfiguration<br>
+-- compared connections by UUID instead of URI.<br>
+--<br>
+util = require('util')<br>
+-- Firstly, clean router from storage_1_a connection.<br>
+rs1_uuid = util.replicasets[1]<br>
+rs1_cfg = cfg.sharding[rs1_uuid]<br>
+cfg.sharding[rs1_uuid] = nil<br>
+vshard.router.cfg(cfg)<br>
+-- Now break the URI in the config.<br>
+old_uri = rs1_cfg.replicas[util.name_to_uuid.storage_1_a].uri<br>
+rs1_cfg.replicas[util.name_to_uuid.storage_1_a].uri = '<a href="https://bad_uri.com:123" rel="noreferrer" target="_blank">https://bad_uri.com:123</a>'<br>
+-- Apply the bad config.<br>
+cfg.sharding[rs1_uuid] = rs1_cfg<br>
+vshard.router.cfg(cfg)<br>
+-- Should fail - master is not available because of the bad URI.<br>
+res, err = vshard.router.callrw(1, 'echo', {1})<br>
+res == nil and err ~= nil<br>
+-- Repair the config.<br>
+rs1_cfg.replicas[util.name_to_uuid.storage_1_a].uri = old_uri<br>
+vshard.router.cfg(cfg)<br>
+-- Should drop the old connection object and connect fine.<br>
+vshard.router.callrw(1, 'echo', {1})<br>
+<br>
_ = test_run:switch("default")<br>
_ = test_run:cmd('stop server router_1')<br>
_ = test_run:cmd('cleanup server router_1')<br>
diff --git a/vshard/replicaset.lua b/vshard/replicaset.lua<br>
index 0191936..b13d05e 100644<br>
--- a/vshard/replicaset.lua<br>
+++ b/vshard/replicaset.lua<br>
@@ -456,7 +456,7 @@ local function rebind_replicasets(replicasets, old_replicasets)<br>
for replica_uuid, replica in pairs(replicaset.replicas) do<br>
local old_replica = old_replicaset and<br>
old_replicaset.replicas[replica_uuid]<br>
- if old_replica then<br>
+ if old_replica and old_replica.uri == replica.uri then<br>
local conn = old_replica.conn<br>
replica.conn = conn<br>
replica.down_ts = old_replica.down_ts<br>
-- <br>
2.21.1 (Apple Git-122.3)<br>
<br>
</blockquote></div>