-
Notifications
You must be signed in to change notification settings - Fork 48
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.
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
(POOLER-129) Allow setting weights for backends #298
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,15 +37,38 @@ def need_token! | |
end | ||
|
||
def fetch_single_vm(template) | ||
vm = backend.spop('vmpooler__ready__' + template) | ||
return [vm, template] if vm | ||
|
||
template_backends = [template] | ||
aliases = Vmpooler::API.settings.config[:alias] | ||
if aliases && aliased_template = aliases[template] | ||
vm = backend.spop('vmpooler__ready__' + aliased_template) | ||
return [vm, aliased_template] if vm | ||
if aliases | ||
template_backends << aliases[template] if aliases[template] | ||
|
||
pool_index = pool_index(pools) | ||
weighted_pools = {} | ||
template_backends.each do |t| | ||
next unless pool_index.key? t | ||
index = pool_index[t] | ||
clone_target = pools[index]['clone_target'] || config['clone_target'] | ||
next unless config.key?('backend_weight') | ||
weight = config['backend_weight'][clone_target] | ||
if weight | ||
weighted_pools[t] = weight | ||
end | ||
end | ||
|
||
if weighted_pools.count == template_backends.count | ||
pickup = Pickup.new(weighted_pools) | ||
selection = pickup.pick | ||
template_backends.delete(selection) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why remove and re-add it at the beginning of the array? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'm following the logic now. Why not capture the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My goal is to order the array so the first VM we try to retrieve is based on the selection from pickup. There is an each statement because we need to try this for the next one in the list in case there are no ready VMs for the first selection. If no pool or matching alias can provide the VM only then will it return nil. |
||
template_backends.unshift(selection) | ||
else | ||
template_backends = template_backends.sample(template_backends.count) | ||
end | ||
end | ||
|
||
template_backends.each do |t| | ||
vm = backend.spop('vmpooler__ready__' + t) | ||
return [vm, t] if vm | ||
end | ||
[nil, nil] | ||
end | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -428,7 +428,7 @@ def destroy_vm(pool_name, vm_name) | |
|
||
def vm_ready?(_pool_name, vm_name) | ||
begin | ||
open_socket(vm_name) | ||
open_socket(vm_name, global_config[:config]['domain']) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this looks like an unrelated change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. I previously implemented the ability to specify a VM domain to use when opening a socket connection. Without this change that domain setting is never used. When interacting with a local instance of vmpooler in docker I found that I could not get VMs to reach the ready state without this addition. I could make this a separate commit if you think that makes more sense. |
||
rescue => _err | ||
return false | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add this
backend_weight
to the docs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.