crush: update choose_args when items are added/removed #15311
Conversation
ghost
requested a review
from 
ghost
changed the title
|
will work on move/reweight now |
Signed-off-by: Loic Dachary <loic@dachary.org>
Signed-off-by: Loic Dachary <loic@dachary.org>
ghost
changed the title
ghost
changed the title
|
@liewegas does that sound reasonable ? I think it addresses what we discussed today. |
| crush_weight_set *weight_set = &arg->weight_set[j]; | ||
| weight_set->weights = (__u32*)realloc(weight_set->weights, new_size * sizeof(__u32)); | ||
| assert(weight_set->size + 1 == new_size); | ||
| weight_set->weights[weight_set->size] = weight; |
There was a problem hiding this comment.
the commit message needs updating? this is setting it to weight arg, not 0
| crush_choose_arg *arg = &arg_map.args[-1-bucket->id]; | ||
| for (__u32 j = 0; j < arg->weight_set_size; j++) { | ||
| crush_weight_set *weight_set = &arg->weight_set[j]; | ||
| weight_set->weights[position] = weight; |
There was a problem hiding this comment.
it might be worth doing the proportional thing here: calculate float ratio = weight / old_weight, and use that ratio to adjust this value.
There was a problem hiding this comment.
The ratio idea is probably correct when the weight of an item is adjusted for the first weight set in a bucket. But the ratio is going to be different for the second (and successive) weight set. We know from experience that the second weight set is going to lower the weight of the items that have the highest target weight and vice versa. It follows that if a target weight changes and becomes higher than another, the ratio needs to be inverted. That's why I think better to just stick with the target weight until rebalancing gets a chance to run.
Am I making things more complicated than they should be ?
src/crush/CrushWrapper.cc
Outdated
| if (choose_args.size() > 0) { | ||
| ldout(cct, 1) << "swap_bucket not implemented when choose_args is not empty" << dendl; | ||
| return -EDOM; | ||
| } |
There was a problem hiding this comment.
i think we can actually support this without much trouble. we're completely swapping the contents, so we can just swap the choose_args for the buckets too. and then call adjust for the bucket itself at the end to account for any net bucket weight change.
|
what's here so far looks good! |
|
Yeah, makes sense!
|
When items are added: - the value in the weight set is set to the target weight. It is assumed that weight set are updated on a regular basis and will eventually be set to a value that prevents excessive over/under filling. - the value in the id list is set to the item id. When items are removed, their weight / ids in all choose_args are removed. Signed-off-by: Loic Dachary <loic@dachary.org>
The osd_crush_update_weight_set (true by default) can be used to disable the update of the weights. Signed-off-by: Loic Dachary <loic@dachary.org>
|
@liewegas the commit message was updated, swap_bucket does not need modification (it already updates the parents weights and that will also update their weight set) and I kept the target weight instead of trying to apply the ratio. Did I miss anything ? |
|
Looks good!
|
Discussion on ceph-devel http://marc.info/?l=ceph-devel&m=149578103618463&w=2