Skip to content

Commit

Permalink
config: don't use an anon replica as an upstream
Browse files Browse the repository at this point in the history
This commit effectively allows to set `replication.anon: true` without
specifying `replication.peers`.

Without filtering out anonymous replicas from the list of upstreams we
get an error regarding attempt to use an anonymous replica as an
upstream for a non-anonymous instance.

Also, anonymous replicas are excluded from autogenerated upstream list
for other anonymous replicas. It makes the list the same on all the
peers.

A user can configure a custom data flow using `replication.peers`
option.

Part of #9432

NO_DOC=The documentation request is in the last commit of the series.
  • Loading branch information
Totktonada committed Dec 6, 2023
1 parent 1f533fc commit 71b69c4
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 19 deletions.
4 changes: 0 additions & 4 deletions changelogs/unreleased/config-anonymous-replica.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@

There are caveats that are not resolved yet:

* An anonymous replica shouldn't be used as an upstream for non-anonymous
replica.
* An anonymous replica possibly shouldn't be used as an upstream for other
anonymous replicas by default.
* An anonymous replica shouldn't be chosen as a bootstrap leader in
`replication.failover: supervised` mode.
* An attempt to configure a replicaset where all instances are anonymous
Expand Down
38 changes: 24 additions & 14 deletions src/box/lua/config/applier/box_cfg.lua
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,6 @@ local snapshot = require('internal.config.utils.snapshot')
local schedule_task = fiber._internal.schedule_task
local mkversion = require('internal.mkversion')

local function peer_uri(configdata, peer_name)
local iconfig = configdata._peers[peer_name].iconfig_def
return instance_config:instance_uri(iconfig, 'peer')
end

local function peer_uris(configdata)
local peers = configdata:peers()
if #peers <= 1 then
Expand All @@ -27,16 +22,31 @@ local function peer_uris(configdata)

local uris = {}
for _, peer_name in ipairs(peers) do
local uri = peer_uri(configdata, peer_name)
if uri == nil then
log.info('%s: instance %q has no iproto.advertise.peer or ' ..
'iproto.listen URI suitable to create a client socket',
err_msg_prefix, peer_name)
end
if uri ~= nil and peer_name ~= names.instance_name then
has_upstream = true
local iconfig_def = configdata._peers[peer_name].iconfig_def
local is_anon = instance_config:get(iconfig_def, 'replication.anon')
-- Don't use anonymous replicas as upstreams.
--
-- An anonymous replica can't be an upstream for a
-- non-anonymous instance.
--
-- An anonymous replica can be an upstream for another
-- anonymous replica, but only non-anonymous peers are set
-- as upstreams by default.
--
-- A user may configure a custom data flow using
-- `replication.peers` option.
if not is_anon then
local uri = instance_config:instance_uri(iconfig_def, 'peer')
if uri == nil then
log.info('%s: instance %q has no iproto.advertise.peer or ' ..
'iproto.listen URI suitable to create a client socket',
err_msg_prefix, peer_name)
end
if uri ~= nil and peer_name ~= names.instance_name then
has_upstream = true
end
table.insert(uris, uri)
end
table.insert(uris, uri)
end

if not has_upstream then
Expand Down
58 changes: 57 additions & 1 deletion test/config-luatest/anonymous_replica_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ g.after_all(replicaset.clean)
--
-- This test excludes the `replication.peers` autoconstruction
-- logic by defining the option on its own. This automatic
-- construction is verified in a separate test case (TODO).
-- construction is verified in a separate test case.
g.test_basic = function(g)
-- One master, two replicas, two anonymous replicas.
local config = cbuilder.new()
Expand Down Expand Up @@ -59,3 +59,59 @@ g.test_basic = function(g)
t.assert_equals(box.info.id, 0)
end)
end

-- Verify that an anonymous replica is not used as an upstream for
-- other instances of the same replicaset.
g.test_no_anonymous_upstream = function(g)
-- One master, two replicas, two anonymous replicas.
--
-- NB: It is the same as config in `test_basic`, but has no
-- `replication.peers` option.
local config = cbuilder.new()
:add_instance('instance-001', {
database = {
mode = 'rw',
},
})
:add_instance('instance-002', {})
:add_instance('instance-003', {})
:add_instance('instance-004', {
replication = {
anon = true,
},
})
:add_instance('instance-005', {
replication = {
anon = true,
},
})
:config()

local replicaset = replicaset.new(g, config)
replicaset:start()

-- Verify that the anonymous replicas are actually anonymous.
replicaset['instance-004']:exec(function()
t.assert_equals(box.info.id, 0)
end)
replicaset['instance-005']:exec(function()
t.assert_equals(box.info.id, 0)
end)

-- Verify box.cfg.replication on all instances.
--
-- NB: We can check box.info.replication as well, but it just
-- adds extra code and doesn't improve coverage of upstream
-- list construction logic.
replicaset:each(function(server)
server:exec(function()
t.assert_equals(box.cfg.replication, {
'replicator:secret@unix/:./instance-001.iproto',
'replicator:secret@unix/:./instance-002.iproto',
'replicator:secret@unix/:./instance-003.iproto',
-- No instance-{004,005}, because they're
-- anonymous replicas.
})
end)
end)
end

0 comments on commit 71b69c4

Please sign in to comment.