Hi Mice,
I think option#2 is better because it will avoid unnecessary iterations and
return when first fit satisfying both volume allocations and host linkage is
found.
However I think it would be good to do all the size computation in a separate
method for readability.
Also following needs to be fixed in your logic:
1) haveEnoughSpace should be initialized to false for every Volume iteration
2) If for some volume none of the pool seems to have enough space (due to
other prior volumes allocated), we need not continue - error out there.
Thus add the check if(!haveEnoughSpace) { //error/ } inside the l
3) storageUsedMap should be initialized for every host iteration, otherwise
while trying the second host 'requestedSpaceSize' might be wrong.
4) Before iterating over the volumes, I think we should sort the list in
descending order of volume size - so that volume needing the largest chunk of
space gets mapped to the pool first.
This will avoid the situation like this:
volA 4G, volb 6G
SP1 6G, SP2 4G
Allocator could return volA -> SP1, SP2 and volB -> SP1.
If you choose the pool for VolA first, then VolB might not find a fit. But if
we sort by size and choose pool for VolB first, we might find a destination.
Thanks,
Prachi
From: Mice Xia [mailto:mice_...@tcloudcomputing.com]
Sent: Wednesday, July 25, 2012 6:43 AM
To: Edison Su
Cc: cloudstack-dev@incubator.apache.org; Prachi Damle; Nitin Mehta
Subject: Re: Review Request: CS-15609 Volumes can be created as a part of VM
creation when un-allocated space is insufficient on primary storage
Hi Edison,
after some further investigation, i found it is non-trivial to put this logic
in findSuitablePoolsForVolumes.
method findSuitablePoolsForVolumes is essentially used for listing all storage
pools for one individual volume.(it even gets a parameter 'returnUpTo'). if it
has to deal with more then one volumes, the computation there will be
exponential, it is basically a tree recursive search for all possible volume
allocation on storagepools, think about following example.
volA 20G , volB 30G
SP1 40G, sp2 60G
there are three possible allocation:
volA->SP1 volB->SP2
volA->SP2 volB->SP2
volA->SP2 volB->SP1
this seems not very efficient and the return signature may need a change.
back to this multiple volumes capacity check issue, i would like to suggest
following two approaches:
1) put special check logic in findSuitablePoolsForVolumes, but, instead of
recursive exploring, return immediately when the first possible allocation is
found.
2) or keep it in findPotentialDeploymentResources, while
findSuitablePoolsForVolumes is responsbile for providing pools for single
volume, findPotentialDeploymentResources will check them as a whole and it will
return immediately after find a proper allocation.
please adivce, thanks
Regards
Mice
