Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

router: add callro methods based on node zone #391

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

router: add callro methods based on node zone #391

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a636535
Add callro methods based on node zone
eugenepaniot Jan 10, 2023
b3656e7
Update comments
eugenepaniot Jan 17, 2023
774dfb2
Use strings as load-balancing algorithm identificator
eugenepaniot Jan 22, 2023
d22797c
Update test/router/router.result add new funct
eugenepaniot Feb 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion vshard/replicaset.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,13 +577,35 @@ local function replicaset_template_multicallro(prefer_replica, balance)
local function pick_next_replica(replicaset, now)
local r
local master = replicaset.master

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, drop not needed empty lines.

if balance then
local prefered_zone = replicaset.priority_list[1].zone
local i = #replicaset.priority_list
while i > 0 do
r = replicaset_balance_replica(replicaset)
i = i - 1
if r:is_connected() and (not prefer_replica or r ~= master) and
replica_check_backoff(r, now) then
replica_check_backoff(r, now)
then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, revert unnecessary changes like these. These 3 lines didn't have to change at all. Besides, we never put then on a separate line. Lets keep it consistent with our code style.

-- Pick a replica according prefered zone (highest priority replica zone) in round-robin manner.
if balance == 2 and prefered_zone then
-- save current rr-cursor position
local cbi = replicaset.balance_i
-- find next replica
local nr = replicaset_balance_replica(replicaset)

if prefered_zone and r.zone and r.zone == prefered_zone -- current replica is in prefered_zone
and nr.zone and nr.zone ~= prefered_zone -- next replica is from different zone (reached prefered_zone replicas)
and (not prefer_replica or nr ~= master)
then
-- reset cursor to the main position
replicaset.balance_i = 1
else
-- restore rr-cursor position
replicaset.balance_i = cbi
end
end

return r
end
end
Expand Down Expand Up @@ -637,6 +659,7 @@ local function replicaset_template_multicallro(prefer_replica, balance)
end
end
opts.timeout = timeout

net_status, storage_status, retval, err =
replica_call(replica, func, args, opts)
now = fiber_clock()
Expand Down Expand Up @@ -990,8 +1013,10 @@ local replicaset_mt = {
callrw = replicaset_master_call;
callro = replicaset_template_multicallro(false, false);
callbro = replicaset_template_multicallro(false, true);
callbzro = replicaset_template_multicallro(false, 2);
callre = replicaset_template_multicallro(true, false);
callbre = replicaset_template_multicallro(true, true);
callbzre = replicaset_template_multicallro(true, 2);
map_call = replicaset_map_call,
update_master = replicaset_update_master,
};
Expand Down Expand Up @@ -1235,6 +1260,7 @@ local function buildall(sharding_cfg)
local function replica_cmp_weight(r1, r2)
-- Master has priority over replicas with the same
-- weight.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, drop this line.

if r1.weight == r2.weight then
return r1 == new_replicaset.master
else
Expand Down
16 changes: 15 additions & 1 deletion vshard/router/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,11 +576,15 @@ local function router_call_impl(router, bucket_id, mode, prefer_replica,
local call
if mode == 'read' then
if prefer_replica then
if balance then
if balance == 2 then
call = 'callbzre'
elseif balance then
call = 'callbre'
else
call = 'callre'
end
elseif balance == 2 then
call = 'callbzro'
elseif balance then
call = 'callbro'
else
Expand Down Expand Up @@ -698,6 +702,10 @@ local function router_callbro(router, bucket_id, ...)
return router_call_impl(router, bucket_id, 'read', false, true, ...)
end

local function router_callbzro(router, bucket_id, ...)
return router_call_impl(router, bucket_id, 'read', false, 2, ...)
end

local function router_callrw(router, bucket_id, ...)
return router_call_impl(router, bucket_id, 'write', false, false, ...)
end
Expand All @@ -710,6 +718,10 @@ local function router_callbre(router, bucket_id, ...)
return router_call_impl(router, bucket_id, 'read', true, true, ...)
end

local function router_callbzre(router, bucket_id, ...)
return router_call_impl(router, bucket_id, 'read', true, 2, ...)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to introduce such feature, we should probably make API more explicit. Using 2 as the value for balance doesn't seem too good. A suppose, the better solution would be to make balance a string, and if its value is something like prefer_zone, balance replicas in a way, you propose. In all other cases of balance, simple rr can be used

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, thank you.

end

local function router_call(router, bucket_id, opts, ...)
local mode, prefer_replica, balance
if opts then
Expand Down Expand Up @@ -1660,6 +1672,8 @@ local router_mt = {
call = router_make_api(router_call),
callro = router_make_api(router_callro),
callbro = router_make_api(router_callbro),
callbzro = router_make_api(router_callbzro),
callbzre = router_make_api(router_callbzre),
callrw = router_make_api(router_callrw),
callre = router_make_api(router_callre),
callbre = router_make_api(router_callbre),
Expand Down