Adding RAID SHR and correcting some bugs :)

[tigerblue77]

No description provided.

[codecov-io]

Codecov Report

Merging #35 into develop will decrease coverage by 34.89%.
The diff coverage is 39.89%.

@@             Coverage Diff              @@
##             develop     #35      +/-   ##
============================================
- Coverage        100%   65.1%   -34.9%     
- Complexity        74     132      +58     
============================================
  Files              8      10       +2     
  Lines            187     341     +154     
============================================
+ Hits             187     222      +35     
- Misses             0     119     +119

Impacted Files	Coverage Δ	Complexity Δ
src/Raid/RaidSHR2.php	0% <0%> (ø)	14 <14> (?)
src/Drive.php	100% <100%> (ø)	11 <0> (-1)	⬇️
src/Raid/RaidFive.php	62.5% <33.33%> (-37.5%)	6 <2> (+1)
src/Raid/RaidZero.php	62.5% <33.33%> (-37.5%)	6 <2> (+2)
src/Raid/RaidSix.php	62.5% <44.44%> (-37.5%)	6 <2> (+1)
src/Raid/RaidOne.php	62.5% <44.44%> (-37.5%)	6 <2> (+2)
src/Raid/RaidTen.php	62.5% <44.44%> (-37.5%)	6 <2> (+2)
src/AbstractRaid.php	76.33% <48.33%> (-23.67%)	58 <39> (+25)
src/Raid/RaidSHR.php	77.27% <77.27%> (ø)	10 <10> (?)
src/RaidFactory.php	92.59% <84.61%> (-7.41%)	9 <9> (+2)
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 56dff07...db9399e. Read the comment docs.

[kevinquinnyo]

@tigerblue77 do you need any help here? I notice there are a few failing tests.

For instance in the RaidTenTest, you changed the capacity of one of the drives in the test, but didn'tupdate the result.

[tigerblue77]

@kevinquinnyo hi, no thank you :) with my studies I didn't have time to repair RAID 10 until now but I'll do ASAP

Failing tests are corrects, I edited them because they were wrong I think ;)

And I changed a drive to make an heterogen drive array but, as there is no pair for this drive size in the array, the RAID10 will only use the size of the second biggest drive

For example if you have 3 1tb drives and one 2tb drive, the raid 10 will create 2 raid 1 of 1 tb usable and 1 tb for mirroring (so 1tb will stay unused on the 2tb drive) and merge them in a RAID 0 (as raid 10 is in fact RAID 1+0)

[kevinquinnyo]

For example if you have 3 1tb drives and one 2tb drive, the raid 10 will create 2 raid 1 of 1 tb usable and 1 tb for mirroring (so 1tb will stay unused on the 2tb drive) and merge them in a RAID 0 (as raid 10 is in fact RAID 1+0)

Oh I see. A RAID 10 is basically invalid with 5 drives. The code is summing the capacity of all 5 (truncating the 2048 to 1024) and resulting in (1024 * 5) / 2 = 2560.

So that is a bug. Good catch. I haven't looked at this code in a long time.

[tigerblue77]

Yes that's it. I think "invalid" is not the right term, as the RAID will be created but as it needs pairs of drives, one drive of the five composing the array will not be used :)
And the algorithm is a little bit more complicated as it will try to make pairs of the same drive capacity to reduce the lost space and maximize the usable space. In fact, in general you only create raid 10 with identical drives as it will have no lost space

Thank you :) no problem

[kevinquinnyo]

I wonder if this library should throw an exception if you pass 5 drives to a raid 10, unless you set one of the drives as a hot spare?

Should hotSpare be a property of a Drive, or should the Raid objects have a $hotSpares (array) property instead?

I never gave this too much thought and I'm not super knowledgable on RAID. What do you think?

edit: my instinct is that we should throw InvalidArgumentException if you pass 5 drives to a RAID10 and one of them is not marked as a hotSpare.

[tigerblue77]

I wonder if this library should throw an exception if you pass 5 drives to a raid 10, unless you set one of the drives as a hot spare?

Sorry but I think it's not a good idea as it is not impossible to build a RAID 10 with 5 drives... 1 drive will stay unused and will not be part of the RAID but it's still possible.
In my opinion, an exception should only be throwed when an impossible case is reached.

Should hotSpare be a property of a Drive, or should the Raid objects have a $hotSpares (array) property instead?

I never gave this too much thought and I'm not super knowledgable on RAID. What do you think?

edit: my instinct is that we should throw InvalidArgumentException if you pass 5 drives to a RAID10 and one of them is not marked as a hotSpare.

No problem :)

I think it should be a drive's property (as boolean) because it is more logical and if it's an array, when a drive is failling, you're obliged to remove the object from the spare array to move it to the used drives array... It's not efficient ;)

[tigerblue77]

I don't know why "Travis CI" check is failing on GitHub when I push... Could you look at this please ? :)

I'm working on RAID 10... The algorithm to calculate the capacity is the same as the one used to calculate de parity size, how would you do to avoid code duplication ? :)

edit: Normally everything is now working great, don't hesitate to ask me for changes or if you don't understand some of my programming choices :)

Do you have ideas of new functionnalities ?

[tigerblue77]

Hi @kevinquinnyo,

Did you have time to look at the changes I made ? :)

[kevinquinnyo]

Did you have time to look at the changes I made ? :)

No I'm really sorry! I have been really busy with work and planning a wedding. I am looking forward to finding some time to look through this and help you land your changes though.

[tigerblue77]

No I'm really sorry! I have been really busy with work and planning a wedding. I am looking forward to finding some time to look through this and help you land your changes though.

No problem ! I'll wait for your return, there is no emergency :)