Randomise selection order for affine blocks. #102
Conversation
|
Are you sure you want this to be random? If you have multiple clients on the same host attempting to allocate IP addresses for the first time you'll end up with multiple allocation blocks being assigned on the host. Wouldn't it be better to use a hash of the host to provide a fix entry point into the CIDRs for a specific host? |
|
Hadn't thought of that, good spot. I'll make that change and put it through its paces in my scale testing. |
fasaxc
force-pushed
the
smc-randomise-block-sel
branch
from
45feede
to
e741c2e
Compare
|
Seems to do the trick after I finally managed to get a working container image. I did spot that there's some pretty similar function in |
fasaxc
force-pushed
the
smc-randomise-block-sel
branch
2 times, most recently
from
a4c2694
to
775edb0
Compare
|
Yeah I've always worried that the shuffling code will be the consuming. Wanna fix that up too? :-) |
fasaxc
force-pushed
the
smc-randomise-block-sel
branch
from
fab36c2
to
bd1bafa
Compare
|
@robbrockbank OK, I've made that change. Means I can now use |
| """Tries to allocate IPs from the explicitly-listed blocks. | ||
|
|
||
| :param list blocks: Blocks to allocate from (for example, the affine | ||
| blocks for a host). |
There was a problem hiding this comment.
nit: formatting off on this line
|
Looks good to me. Couple of minor comments. Took me a while to get my head round the random blocks thing... not sure if because I'm slow, or whether it just needs some beefing up of the comments. |
fasaxc
force-pushed
the
smc-randomise-block-sel
branch
from
bd1bafa
to
66a8708
Compare
|
Added more comments, and the license for netaddr. Bit of a shame to have to pick up more license than code but I think that's necessary. |
|
lol - wish you'd put that comment in there sooner - I had to get my maths head on... it's been a long time ;-) Looks good. Need to resolve conflicts though, then we're good to go. |
Avoids contention when many hosts assign blocks concurrently. - Rework IPAMClient.random_blocks() to avoid collecting the whole list. Instead use _random_subnets_from_cidrs() to generate blocks form the list of CIDRs.
fasaxc
force-pushed
the
smc-randomise-block-sel
branch
2 times, most recently
from
13685d0
to
f0f9f33
Compare
I think this is what's needed to avoid contention when many hosts assign IPAM blocks concurrently. I'm just about to try it in a scale test, though...