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

fix(balancer) respect the given balancer size #2747

Merged
merged 1 commit into from
Aug 31, 2017
Merged

fix(balancer) respect the given balancer size #2747

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions kong/core/balancer.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ local get_balancer = function(target)
if not balancer then
-- no balancer yet (or invalidated) so create a new one
balancer, err = ring_balancer.new({
wheelsize = upstream.slots,
wheelSize = upstream.slots,
Copy link
Member

Choose a reason for hiding this comment

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

It seems like this value isn't used by the current version of the balancer lib:

  if (not opts.wheelSize) and opts.order then
    opts.wheelSize = #opts.order
  end
  if opts.order then
    assert(opts.order and (opts.wheelSize == #opts.order), "mismatch between size of 'order' and 'wheelSize'")
  end

From the above snippet, we can see that if wheelSize isn't given (which is the case prior to this change), then it is inferred from opts.order, which is given. Hence, while this fix is a good catch, it isn't necessary for 0.11.0 or 0.10.4 stable releases (in fact, it is slightly riskier to include it in those).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, you are right, just double checked it. I ran into it when ripping out the orderlist property. But it seems that that change actually made this bug surface, as it was hidden by the fact that the orderlist was always given.

order = upstream.orderlist,
dns = dns_client,
})
Expand Down Expand Up @@ -219,7 +219,7 @@ local get_balancer = function(target)
-- and can replay changes, but not supported by ring-balancer yet.
-- for now; create a new balancer from scratch
balancer, err = ring_balancer.new({
wheelsize = upstream.slots,
wheelSize = upstream.slots,
order = upstream.orderlist,
dns = dns_client,
})
Expand Down