(POOLER-129) Allow setting weights for backends #298
Conversation
This commit updates get_vm in the vmpooler API to allow for setting weights for backends. Additionally, when an alias for a pool exists, and the backend configured is not weighted, then the selection of the pool based on alias will be randomly sampled. Without this change any pool with the title of the alias is exhausted before an alternate pool with the configured alias is used, which results in an uneven distribution of VMs. When all backends involved are configured with weighted values the VM selection will be based on probability using those weights. A bug is fixed when setting the default ttl for check_ready_vm. Pickup is added to handle weighted VM selection. A dockerfile is added that allows for building and installing vmpooler from the current HEAD in docker to make for easy testing.
| 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') |
There was a problem hiding this comment.
We should add this backend_weight to the docs.
| @@ -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.
this looks like an unrelated change?
There was a problem hiding this comment.
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.
lib/vmpooler/api/v1.rb
Outdated
| def fetch_single_vm(template) | ||
| vm = backend.spop('vmpooler__ready__' + template) | ||
| return [vm, template] if vm | ||
| def pool_backend(pool) |
There was a problem hiding this comment.
a similar method already exists as a helper method and used 4-5 times in the code
vmpooler/lib/vmpooler/providers/vsphere.rb
Lines 451 to 459 in 2daa524
There was a problem hiding this comment.
This method was temporary and isn't used. I need to push the change that removes it. Sorry for the noise.
| 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.
why remove and re-add it at the beginning of the array?
There was a problem hiding this comment.
I think I'm following the logic now. Why not capture the selection here and in the else below. Then instead of doing template_backends.each on line 75 again, just backend.spop() the selection?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I'm 👍 on this, but a challenge I had is that I'm not sure I understand how the weight numbers work from the docs.
They seem to imply that the weights should add to 100, but it doesn't appear true. If you look to the pickup docs, it shows that they don't have to add to 100, but I assume they're normalized to 100% by having whatever number they add to be be the denominator of the fraction?
|
From what I understand with reading the pickup docs you are setting a probability with that value that is relative to the other values, but has no requirements for equalling a total value, or anything like that. Do you think the example would be more clear if single digits, or values that do not total 100 were used? |
This commit updates get_vm in the vmpooler API to allow for setting weights for backends. Additionally, when an alias for a pool exists, and the backend configured is not weighted, then the selection of the pool based on alias will be randomly sampled. Without this change any pool with the title of the alias is exhausted before an alternate pool with the configured alias is used, which results in an uneven distribution of VMs. When all backends involved are configured with weighted values the VM selection will be based on probability using those weights.
A bug is fixed when setting the default ttl for check_ready_vm.
Pickup is added to handle weighted VM selection.
A dockerfile is added that allows for building and installing vmpooler
from the current HEAD in docker to make for easy testing.